[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile
ioeric added a comment. In https://reviews.llvm.org/D49197#1158972, @simark wrote: > Background: I'm trying to fix the cases why we receive a FileEntry without a > real path computed in clangd, so we can avoid having to compute it ourselves. I'm curious how you use `getVirtualFile` in your clangd tests. In general, I'd highly recommend virtual file system in favor of remapped files for clangd tests. Comment at: lib/Basic/FileManager.cpp:395 + SmallString<128> RealPathName; + if (!FS->getRealPath(InterndFileName, RealPathName)) +UFE->RealPathName = RealPathName.str(); simark wrote: > ioeric wrote: > > It seems that at this point, we have failed to find `FileName` in vfs (with > > `getStatValue` above), so `getRealPath` would also fail? > > > > In general, the virtual file here can be an actual *virtual* file that > > doesn't exist anywhere, and I think there are users who use this to map > > virtual file (possibly with relative paths) into file manager (while they > > should really use overlay vfs!). > > It seems that at this point, we have failed to find FileName in vfs (with > > getStatValue above), so getRealPath would also fail? > > From what I understood, this code will be executed if the file actually > exists but it's the first time we access it (we won't take the return at line > 373). If we take the return at line 373, then some previous invocation of > getFile or getVirtualFile has created the file, and that invocation would > have been responsible for computing the real path. > > > In general, the virtual file here can be an actual *virtual* file that > > doesn't exist anywhere, and I think there are users who use this to map > > virtual file (possibly with relative paths) into file manager (while they > > should really use overlay vfs!). > > My thinking was that the worst that could happen is that `getRealPath` fails > in that case. > From what I understood, this code will be executed if the file actually > exists but it's the first time we access it (we won't take the return at line > 373). If we take the return at line 373, then some previous invocation of > getFile or getVirtualFile has created the file, and that invocation would > have been responsible for computing the real path. I see. Thanks for the explanation! > My thinking was that the worst that could happen is that getRealPath fails in > that case. It might make sense to take a look at how `getVirtualFile` is used in clang. For example, in CompilerInstance, it's used to create virtual FileEntry for remapped files (https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CompilerInstance.cpp#L329), and contents will be overwritten in `SourceManager`. This usage is arguable as we have overlay file system nowadays. But in this case, if we try to `getRealPath` on a remapped file, it might happen that we accidentally get an absolute path on the real file system, if there happens to be a file with the same path (relative to the CWD) on disk. This can be surprising and could be hard to debug. This is not unusual as people tend to use file remapping to overwrite the content of disk files with dirty buffers. In general, virtual files in FileManager and virtual files in vfs are different things, and mixing them up further would likely cause more confusion in the future. We should really get rid of the remapped virtual files in `FileManager` and implement them properly with an overlay fs... Repository: rC Clang https://reviews.llvm.org/D49197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336890 - [clangd] Simplify logging wrapper after r336888
Author: sammccall Date: Thu Jul 12 01:00:21 2018 New Revision: 336890 URL: http://llvm.org/viewvc/llvm-project?rev=336890&view=rev Log: [clangd] Simplify logging wrapper after r336888 Modified: clang-tools-extra/trunk/clangd/Logger.h Modified: clang-tools-extra/trunk/clangd/Logger.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Logger.h?rev=336890&r1=336889&r2=336890&view=diff == --- clang-tools-extra/trunk/clangd/Logger.h (original) +++ clang-tools-extra/trunk/clangd/Logger.h Thu Jul 12 01:00:21 2018 @@ -13,6 +13,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Error.h" +#include "llvm/Support/FormatAdapters.h" #include "llvm/Support/FormatVariadic.h" namespace clang { @@ -35,21 +36,11 @@ const char *debugType(const char *Filena void log(Logger::Level, const llvm::formatv_object_base &); // We often want to consume llvm::Errors by value when passing them to log(). -// This is tricky because the logging infrastructure must mark them as handled. -// When forwarding argument to formatv, we wrap Errors-by-value in this type -// whose destructor handles the cleanup. -// FIXME: simplify after D49170 lands. -struct WrappedError { - llvm::Error E; - WrappedError(WrappedError &&) = default; - ~WrappedError() { consumeError(std::move(E)); } -}; -inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, - const WrappedError &Err) { - return OS << Err.E; -} +// We automatically wrap them in llvm::fmt_consume() as formatv requires. template T &&wrap(T &&V) { return std::forward(V); } -inline WrappedError wrap(llvm::Error &&V) { return WrappedError{std::move(V)}; } +inline decltype(fmt_consume(llvm::Error::success())) wrap(llvm::Error &&V) { + return fmt_consume(std::move(V)); +} template void log(Logger::Level L, const char *Fmt, Ts &&... Vals) { detail::log(L, llvm::formatv(Fmt, detail::wrap(std::forward(Vals))...)); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
gerazo added a comment. @martong I don't have commit rights. Thanks for your help in advance. https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49223: [AST] Check described template at structural equivalence check.
balazske created this revision. Herald added a subscriber: cfe-commits. When checking a class or function the described class or function template is checked too. Improved test with symmetric check, added new tests. Repository: rC Clang https://reviews.llvm.org/D49223 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/StructuralEquivalenceTest.cpp Index: unittests/AST/StructuralEquivalenceTest.cpp === --- unittests/AST/StructuralEquivalenceTest.cpp +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -63,10 +63,18 @@ } bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) { -llvm::DenseSet> NonEquivalentDecls; -StructuralEquivalenceContext Ctx(D0->getASTContext(), D1->getASTContext(), - NonEquivalentDecls, false, false); -return Ctx.IsStructurallyEquivalent(D0, D1); +llvm::DenseSet> NonEquivalentDecls01; +llvm::DenseSet> NonEquivalentDecls10; +StructuralEquivalenceContext Ctx01( +D0->getASTContext(), D1->getASTContext(), +NonEquivalentDecls01, false, false); +StructuralEquivalenceContext Ctx10( +D1->getASTContext(), D0->getASTContext(), +NonEquivalentDecls10, false, false); +bool Eq01 = Ctx01.IsStructurallyEquivalent(D0, D1); +bool Eq10 = Ctx10.IsStructurallyEquivalent(D1, D0); +EXPECT_EQ(Eq01, Eq10); +return Eq01; } bool testStructuralMatch(std::tuple t) { @@ -199,6 +207,14 @@ struct StructuralEquivalenceFunctionTest : StructuralEquivalenceTest { }; +TEST_F(StructuralEquivalenceFunctionTest, TemplateVsNonTemplate) { + auto t = makeNamedDecls( + "void foo();", + "template void foo();", + Lang_CXX); + EXPECT_FALSE(testStructuralMatch(t)); +} + TEST_F(StructuralEquivalenceFunctionTest, ParamConstWithRef) { auto t = makeNamedDecls("void foo(int&);", "void foo(const int&);", Lang_CXX); @@ -534,6 +550,15 @@ EXPECT_TRUE(testStructuralMatch(t)); } +TEST_F(StructuralEquivalenceRecordTest, TemplateVsNonTemplate) { + auto t = makeDecls( + "struct A { };", + "template struct A { };", + Lang_CXX, + cxxRecordDecl(hasName("A"))); + EXPECT_FALSE(testStructuralMatch(t)); +} + TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) { auto t = makeNamedDecls( "struct A{ }; struct B{ }; void foo(A a, A b);", Index: lib/AST/ASTStructuralEquivalence.cpp === --- lib/AST/ASTStructuralEquivalence.cpp +++ lib/AST/ASTStructuralEquivalence.cpp @@ -955,13 +955,21 @@ if (!D1 || !D2) return true; + if (D1->isTemplated() != D2->isTemplated()) +return false; + if (auto *D1CXX = dyn_cast(D1)) { if (auto *D2CXX = dyn_cast(D2)) { if (D1CXX->hasExternalLexicalStorage() && !D1CXX->isCompleteDefinition()) { D1CXX->getASTContext().getExternalSource()->CompleteType(D1CXX); } + if (auto *T1 = D1CXX->getDescribedClassTemplate()) +if (auto *T2 = D2CXX->getDescribedClassTemplate()) + if (!IsStructurallyEquivalent(Context, T1, T2)) +return false; + if (D1CXX->getNumBases() != D2CXX->getNumBases()) { if (Context.Complain) { Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent) @@ -1309,6 +1317,14 @@ if (!IsStructurallyEquivalent(Context, D1->getType(), D2->getType())) return false; + if (D1->isTemplated() != D2->isTemplated()) +return false; + + if (auto T1 = D1->getDescribedFunctionTemplate()) +if (auto T2 = D2->getDescribedFunctionTemplate()) + if (!IsStructurallyEquivalent(Context, T1, T2)) +return false; + return true; } Index: unittests/AST/StructuralEquivalenceTest.cpp === --- unittests/AST/StructuralEquivalenceTest.cpp +++ unittests/AST/StructuralEquivalenceTest.cpp @@ -63,10 +63,18 @@ } bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) { -llvm::DenseSet> NonEquivalentDecls; -StructuralEquivalenceContext Ctx(D0->getASTContext(), D1->getASTContext(), - NonEquivalentDecls, false, false); -return Ctx.IsStructurallyEquivalent(D0, D1); +llvm::DenseSet> NonEquivalentDecls01; +llvm::DenseSet> NonEquivalentDecls10; +StructuralEquivalenceContext Ctx01( +D0->getASTContext(), D1->getASTContext(), +NonEquivalentDecls01, false, false); +StructuralEquivalenceContext Ctx10( +D1->getASTContext(), D0->getASTContext(), +NonEquivalentDecls10, false, false); +bool Eq01 = Ctx01.IsStructurallyEquivalent(D0, D1); +bool Eq10 = Ctx10.IsStructurallyEquivalent(D1, D0); +EXPECT_EQ(Eq01, Eq10); +return Eq01; } bool testStructuralMatch(std::tuple t) { @@ -199,6 +207,14 @@ struct StructuralEquivalenceFunctionTest : StructuralEquivalenceTes
[PATCH] D49224: [clangd] log request/response messages with method/ID/error at INFO level
sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. Bodies are logged at VERBOSE level (since r336785), tweak the formatting. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49224 Files: clangd/JSONRPCDispatcher.cpp Index: clangd/JSONRPCDispatcher.cpp === --- clangd/JSONRPCDispatcher.cpp +++ clangd/JSONRPCDispatcher.cpp @@ -69,7 +69,7 @@ Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S; Outs.flush(); } - vlog("--> {0}\n", S); + vlog(">>> {0}\n", S); } void JSONOutput::log(Logger::Level Level, @@ -99,6 +99,7 @@ return; } RequestSpan::attach([&](json::Object &Args) { Args["Reply"] = Result; }); + log("--> reply({0})", *ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -116,6 +117,7 @@ }); if (auto ID = Context::current().get(RequestID)) { +log("--> reply({0}) error: {1}", *ID, Message); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -133,11 +135,13 @@ RequestSpan::attach([&](json::Object &Args) { Args["Call"] = json::Object{{"method", Method.str()}, {"params", Params}}; }); + auto ID = 1; + log("--> {0}({1})", Method, ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ {"jsonrpc", "2.0"}, - {"id", 1}, + {"id", ID}, {"method", Method}, {"params", std::move(Params)}, }); @@ -148,6 +152,23 @@ Handlers[Method] = std::move(H); } +static void logIncomingMessage(const llvm::Optional &ID, + llvm::Optional Method, + const json::Object *Error) { + if (Method) { // incoming request +if (ID) // call + log("<-- {0}({1})", *Method, *ID); +else // notification + log("<-- {0}", *Method); + } else if (ID) { // response, ID must be provided +if (Error) + log("<-- reply({0}) error: {1}", *ID, + Error->getString("message").getValueOr("")); +else + log("<-- reply({0})", *ID); + } +} + bool JSONRPCDispatcher::call(const json::Value &Message, JSONOutput &Out) const { // Message must be an object with "jsonrpc":"2.0". @@ -158,9 +179,9 @@ llvm::Optional ID; if (auto *I = Object->get("id")) ID = std::move(*I); - // Method must be given. auto Method = Object->getString("method"); - if (!Method) + logIncomingMessage(ID, Method, Object->getObject("error")); + if (!Method) // We only handle incoming requests, and ignore responses. return false; // Params should be given, use null if not. json::Value Params = nullptr; @@ -333,13 +354,13 @@ if (auto JSON = ReadMessage(In, Out)) { if (auto Doc = json::parse(*JSON)) { // Log the formatted message. -vlog(Out.Pretty ? "<-- {0:2}\n" : "<-- {0}\n", *Doc); +vlog(Out.Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc); // Finally, execute the action for this JSON message. if (!Dispatcher.call(*Doc, Out)) elog("JSON dispatch failed!"); } else { // Parse error. Log the raw message. -vlog("<-- {0}\n", *JSON); +vlog("<<< {0}\n", *JSON); elog("JSON parse error: {0}", llvm::toString(Doc.takeError())); } } Index: clangd/JSONRPCDispatcher.cpp === --- clangd/JSONRPCDispatcher.cpp +++ clangd/JSONRPCDispatcher.cpp @@ -69,7 +69,7 @@ Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S; Outs.flush(); } - vlog("--> {0}\n", S); + vlog(">>> {0}\n", S); } void JSONOutput::log(Logger::Level Level, @@ -99,6 +99,7 @@ return; } RequestSpan::attach([&](json::Object &Args) { Args["Reply"] = Result; }); + log("--> reply({0})", *ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -116,6 +117,7 @@ }); if (auto ID = Context::current().get(RequestID)) { +log("--> reply({0}) error: {1}", *ID, Message); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -133,11 +135,13 @@ RequestSpan::attach([&](json::Object &Args) { Args["Call"] = json::Object{{"method", Method.str()}, {"params", Params}}; }); + auto ID = 1; + log("--> {0}({1})", Method, ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ {"jsonrpc", "2.0"}, - {"id", 1}, + {"id", ID}, {"method", Method}, {"params", std::move(Params)}, }); @@ -148,6 +152,23 @@ Handlers[Method] = std::move(H); } +static void logIncomingMessage(const llvm::Optional &ID, + llvm::Optional Method, +
r336896 - [ASTImporter] Refactor Decl creation
Author: martong Date: Thu Jul 12 02:42:05 2018 New Revision: 336896 URL: http://llvm.org/viewvc/llvm-project?rev=336896&view=rev Log: [ASTImporter] Refactor Decl creation Summary: Generalize the creation of Decl nodes during Import. With this patch we do the same things after and before a new AST node is created (::Create) The import logic should be really simple, we create the node, then we mark that as imported, then we recursively import the parts for that node and then set them on that node. However, the AST is actually a graph, so we have to handle circles. If we mark something as imported (`MapImported()`) then we return with the corresponding `To` decl whenever we want to import that node again, this way circles are handled. In order to make this algorithm work we must ensure things, which are handled in the generic CreateDecl<> template: * There are no `Import()` calls in between any node creation (::Create) and the `MapImported()` call. * Before actually creating an AST node (::Create), we must check if the Node had been imported already, if yes then return with that one. One very important case for this is connected to templates: we may start an import both from the templated decl of a template and from the template itself. Now, the virtual `Imported` function is called in `ASTImporter::Impor(Decl *)`, but only once, when the `Decl` is imported. One point of this refactor is to separate responsibilities. The original `Imported()` had 3 responsibilities: - notify subclasses when an import happened - register the decl into `ImportedDecls` - initialise the Decl (set attributes, etc) Now all of these are in separate functions: - `Imported` - `MapImported` - `InitializeImportedDecl` I tried to check all the clients, I executed tests for `ExternalASTMerger.cpp` and some unittests for lldb. Reviewers: a.sidorin, balazske, xazax.hun, r.stahl Subscribers: rnkovacs, dkrupp, cfe-commits Differential Revision: https://reviews.llvm.org/D47632 Modified: cfe/trunk/include/clang/AST/ASTImporter.h cfe/trunk/include/clang/AST/ASTStructuralEquivalence.h cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp cfe/trunk/lib/AST/ExternalASTMerger.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp Modified: cfe/trunk/include/clang/AST/ASTImporter.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=336896&r1=336895&r2=336896&view=diff == --- cfe/trunk/include/clang/AST/ASTImporter.h (original) +++ cfe/trunk/include/clang/AST/ASTImporter.h Thu Jul 12 02:42:05 2018 @@ -314,13 +314,13 @@ class Attr; /// \param D A declaration in the "to" context. virtual void CompleteDecl(Decl* D); -/// Note that we have imported the "from" declaration by mapping it -/// to the (potentially-newly-created) "to" declaration. -/// /// Subclasses can override this function to observe all of the \c From -> /// \c To declaration mappings as they are imported. -virtual Decl *Imported(Decl *From, Decl *To); - +virtual Decl *Imported(Decl *From, Decl *To) { return To; } + +/// Store and assign the imported declaration to its counterpart. +Decl *MapImported(Decl *From, Decl *To); + /// Called by StructuralEquivalenceContext. If a RecordDecl is /// being compared to another RecordDecl as part of import, completing the /// other RecordDecl may trigger importation of the first RecordDecl. This Modified: cfe/trunk/include/clang/AST/ASTStructuralEquivalence.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTStructuralEquivalence.h?rev=336896&r1=336895&r2=336896&view=diff == --- cfe/trunk/include/clang/AST/ASTStructuralEquivalence.h (original) +++ cfe/trunk/include/clang/AST/ASTStructuralEquivalence.h Thu Jul 12 02:42:05 2018 @@ -30,6 +30,14 @@ class QualType; class RecordDecl; class SourceLocation; +/// \brief Whether to perform a normal or minimal equivalence check. +/// In case of `Minimal`, we do not perform a recursive check of decls with +/// external storage. +enum class StructuralEquivalenceKind { + Default, + Minimal, +}; + struct StructuralEquivalenceContext { /// AST contexts for which we are checking structural equivalence. ASTContext &FromCtx, &ToCtx; @@ -47,6 +55,8 @@ struct StructuralEquivalenceContext { /// (which we have already complained about). llvm::DenseSet> &NonEquivalentDecls; + StructuralEquivalenceKind EqKind; + /// Whether we're being strict about the spelling of types when /// unifying two types. bool StrictTypeSpelling; @@ -63,10 +73,11 @@ struct StructuralEquivalenceContext { StructuralEquivalenceContext( A
[PATCH] D47632: [ASTImporter] Refactor Decl creation
This revision was automatically updated to reflect the committed changes. Closed by commit rL336896: [ASTImporter] Refactor Decl creation (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47632?vs=154582&id=155134#toc Repository: rL LLVM https://reviews.llvm.org/D47632 Files: cfe/trunk/include/clang/AST/ASTImporter.h cfe/trunk/include/clang/AST/ASTStructuralEquivalence.h cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp cfe/trunk/lib/AST/ExternalASTMerger.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -90,11 +90,83 @@ return getCanonicalForwardRedeclChain(FD); } + void updateFlags(const Decl *From, Decl *To) { +// Check if some flags or attrs are new in 'From' and copy into 'To'. +// FIXME: Other flags or attrs? +if (From->isUsed(false) && !To->isUsed(false)) + To->setIsUsed(); + } + class ASTNodeImporter : public TypeVisitor, public DeclVisitor, public StmtVisitor { ASTImporter &Importer; +// Wrapper for an overload set. +template struct CallOverloadedCreateFun { + template + auto operator()(Args &&... args) + -> decltype(ToDeclT::Create(std::forward(args)...)) { +return ToDeclT::Create(std::forward(args)...); + } +}; + +// Always use these functions to create a Decl during import. There are +// certain tasks which must be done after the Decl was created, e.g. we +// must immediately register that as an imported Decl. The parameter `ToD` +// will be set to the newly created Decl or if had been imported before +// then to the already imported Decl. Returns a bool value set to true if +// the `FromD` had been imported before. +template +LLVM_NODISCARD bool GetImportedOrCreateDecl(ToDeclT *&ToD, FromDeclT *FromD, +Args &&... args) { + // There may be several overloads of ToDeclT::Create. We must make sure + // to call the one which would be chosen by the arguments, thus we use a + // wrapper for the overload set. + CallOverloadedCreateFun OC; + return GetImportedOrCreateSpecialDecl(ToD, OC, FromD, +std::forward(args)...); +} +// Use this overload if a special Type is needed to be created. E.g if we +// want to create a `TypeAliasDecl` and assign that to a `TypedefNameDecl` +// then: +// TypedefNameDecl *ToTypedef; +// GetImportedOrCreateDecl(ToTypedef, FromD, ...); +template +LLVM_NODISCARD bool GetImportedOrCreateDecl(ToDeclT *&ToD, FromDeclT *FromD, +Args &&... args) { + CallOverloadedCreateFun OC; + return GetImportedOrCreateSpecialDecl(ToD, OC, FromD, +std::forward(args)...); +} +// Use this version if a special create function must be +// used, e.g. CXXRecordDecl::CreateLambda . +template +LLVM_NODISCARD bool +GetImportedOrCreateSpecialDecl(ToDeclT *&ToD, CreateFunT CreateFun, + FromDeclT *FromD, Args &&... args) { + ToD = cast_or_null(Importer.GetAlreadyImportedOrNull(FromD)); + if (ToD) +return true; // Already imported. + ToD = CreateFun(std::forward(args)...); + InitializeImportedDecl(FromD, ToD); + return false; // A new Decl is created. +} + +void InitializeImportedDecl(Decl *FromD, Decl *ToD) { + Importer.MapImported(FromD, ToD); + ToD->IdentifierNamespace = FromD->IdentifierNamespace; + if (FromD->hasAttrs()) +for (const Attr *FromAttr : FromD->getAttrs()) + ToD->addAttr(Importer.Import(FromAttr)); + if (FromD->isUsed()) +ToD->setIsUsed(); + if (FromD->isImplicit()) +ToD->setImplicit(); +} + public: explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {} @@ -1485,6 +1557,12 @@ return false; } +static StructuralEquivalenceKind +getStructuralEquivalenceKind(const ASTImporter &Importer) { + return Importer.isMinimalImport() ? StructuralEquivalenceKind::Minimal +: StructuralEquivalenceKind::Default; +} + bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord, bool Complain) { // Eliminate a potential failure point where we attempt to re-import @@ -1499,37 +1577,41 @@ StructuralEquivalenceContext Ctx(Importer.getFromContext(),
[PATCH] D48722: [ASTImporter] Update isUsed flag at Decl import.
martong abandoned this revision. martong added a comment. Closed in favor of https://reviews.llvm.org/D47632 Repository: rC Clang https://reviews.llvm.org/D48722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49227: Override a bit fields layout from an external source
aleksandr.urakov created this revision. aleksandr.urakov added reviewers: rsmith, mstorsjo, arphaman, whunt, majnemer. aleksandr.urakov added a project: clang. This patch adds a possibility to use an external layout for bit fields. The problem occurred while debugging a Windows application with PDB symbols. The structure was as follows: struct S { short a : 3; short : 8; short b : 5; } The problem is that PDB doesn't have information about the unnamed bit field, so the field `b` was located just behind the field `a`. But PDB has a valid information about fields offsets, so we can use it to solve the problem. Repository: rC Clang https://reviews.llvm.org/D49227 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/Inputs/override-bit-field-layout.layout test/CodeGenCXX/override-bit-field-layout.cpp Index: test/CodeGenCXX/override-bit-field-layout.cpp === --- /dev/null +++ test/CodeGenCXX/override-bit-field-layout.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s + +// CHECK: Type: struct S +// CHECK: FieldOffsets: [0, 11] +struct S { + short a : 3; + short b : 5; +}; + +void use_structs() { + S ss[sizeof(S)]; +} Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout === --- /dev/null +++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout @@ -0,0 +1,8 @@ + +*** Dumping AST Record Layout +Type: struct S + +Layout: Index: lib/AST/RecordLayoutBuilder.cpp === --- lib/AST/RecordLayoutBuilder.cpp +++ lib/AST/RecordLayoutBuilder.cpp @@ -2677,7 +2677,7 @@ // Check to see if this bitfield fits into an existing allocation. Note: // MSVC refuses to pack bitfields of formal types with different sizes // into the same allocation. - if (!IsUnion && LastFieldIsNonZeroWidthBitfield && + if (!UseExternalLayout && !IsUnion && LastFieldIsNonZeroWidthBitfield && CurrentBitfieldSize == Info.Size && Width <= RemainingBitsInField) { placeFieldAtBitOffset(Context.toBits(Size) - RemainingBitsInField); RemainingBitsInField -= Width; @@ -2689,6 +2689,14 @@ placeFieldAtOffset(CharUnits::Zero()); Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. + } else if (UseExternalLayout) { +auto FieldBitOffset = External.getExternalFieldOffset(FD); +placeFieldAtBitOffset(FieldBitOffset); +auto NewSize = Context.toCharUnitsFromBits( +llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth())); +assert(NewSize >= Size && "bit field offset already allocated"); +Size = NewSize; +Alignment = std::max(Alignment, Info.Alignment); } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); Index: test/CodeGenCXX/override-bit-field-layout.cpp === --- /dev/null +++ test/CodeGenCXX/override-bit-field-layout.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s + +// CHECK: Type: struct S +// CHECK: FieldOffsets: [0, 11] +struct S { + short a : 3; + short b : 5; +}; + +void use_structs() { + S ss[sizeof(S)]; +} Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout === --- /dev/null +++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout @@ -0,0 +1,8 @@ + +*** Dumping AST Record Layout +Type: struct S + +Layout: Index: lib/AST/RecordLayoutBuilder.cpp === --- lib/AST/RecordLayoutBuilder.cpp +++ lib/AST/RecordLayoutBuilder.cpp @@ -2677,7 +2677,7 @@ // Check to see if this bitfield fits into an existing allocation. Note: // MSVC refuses to pack bitfields of formal types with different sizes // into the same allocation. - if (!IsUnion && LastFieldIsNonZeroWidthBitfield && + if (!UseExternalLayout && !IsUnion && LastFieldIsNonZeroWidthBitfield && CurrentBitfieldSize == Info.Size && Width <= RemainingBitsInField) { placeFieldAtBitOffset(Context.toBits(Size) - RemainingBitsInField); RemainingBitsInField -= Width; @@ -2689,6 +2689,14 @@ placeFieldAtOffset(CharUnits::Zero()); Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. + } else if (UseExternalLayout) { +auto FieldBitOffset = External.getExternalFieldOffset(FD); +placeFieldAtBitOffset(FieldBitOffset); +auto NewSize = Context.toCharUnitsFromBits( +llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth())); +
[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line
djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:760 (!Style.AllowAllParametersOfDeclarationOnNextLine && State.Line->MustBeDeclaration) || +(!Style.AllowAllArgumentsOnNextLine && russellmcc wrote: > djasper wrote: > > This still looks suspicious to me. State.Line->MustBeDeclaration is either > > true or false meaning that AllowAllParametersOfDeclarationOnNextLine or > > AllowAllArgumentsOnNextLine can always affect the behavior here, leading to > > BreakBeforeParameter to be set to true, even if we are in the case for > > PreviousIsBreakingCtorInitializerColon being true. > > > > So, my guess would be that if you set one of AllowAllArgumentsOnNextLine > > and AllowAllParametersOfDeclarationOnNextLine to false, then > > AllowAllConstructorInitializersOnNextLine doesn't have an effect anymore. > > > > Please verify, and if this is true, please fix and add tests. I think this > > might be easier to understand if you pulled the one if statement apart. > Actually, I think this logic works. The case you are worried about > (interaction between arguments, parameters, and constructor initializers) is > already tested in the unit tests in the > `AllowAllConstructorInitializersOnNextLine` test. The specific concern you > have is solved by the separate if statement below. > > I agree that this logic is a bit complex, but I think it's necessary since in > most cases we don't want to change the existing value of > `State.Stack.back().BreakBeforeParameter` - we only want to change this when > we are sure we should or shouldn't bin-pack. I've tried hard not to change > any existing behavior unless it was clearly a bug. I think we could simplify > this logic if we wanted to be less conservative. > > I'm not sure what you mean by breaking up the if statement - did you mean > something like this? To me, this reads much more verbose and is a bit more > confusing; however I'm happy to edit the diff if it makes more sense to you: > > ``` > // If we are breaking after '(', '{', '<', or this is the break after a > ':' > // to start a member initializater list in a constructor, this should not > // be considered bin packing unless the relevant AllowAll option is false > or > // this is a dict/object literal. > bool PreviousIsBreakingCtorInitializerColon = > Previous.is(TT_CtorInitializerColon) && > Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon; > > if (!(Previous.isOneOf(tok::l_paren, tok::l_brace, TT_BinaryOperator) || > PreviousIsBreakingCtorInitializerColon)) > State.Stack.back().BreakBeforeParameter = true; > > if (!Style.AllowAllParametersOfDeclarationOnNextLine && > State.Line->MustBeDeclaration) > State.Stack.back().BreakBeforeParameter = true; > > if (!Style.AllowAllArgumentsOnNextLine && !State.Line->MustBeDeclaration) > State.Stack.back().BreakBeforeParameter = true; > > if (!Style.AllowAllConstructorInitializersOnNextLine && > PreviousIsBreakingCtorInitializerColon) > State.Stack.back().BreakBeforeParameter = true; > > if (Previous.is(TT_DictLiteral))) > State.Stack.back().BreakBeforeParameter = true; > > // If we are breaking after a ':' to start a member initializer list, > // and we allow all arguments on the next line, we should not break > // before the next parameter. > if (PreviousIsBreakingCtorInitializerColon && > Style.AllowAllConstructorInitializersOnNextLine) > State.Stack.back().BreakBeforeParameter = false; > ``` I find it hard to say whether you actually have a test for this. I'll make a suggestion on how to make these tests more maintainable below. I understand now how this works, but the specific case I was worried about is: AllowAllConstructorInitializersOnNextLine = true AllowAllArgumentsOnNextLine = false AllowAllParametersOfDeclarationOnNextLine = false (likely you only need one of the latter, but I am not sure which one :) ) This works, because you are overwriting the value again in the subsequent if statement (which I hadn't seen). However, I do personally find that logic hard to reason about (if you have a sequence of if statements where some of them might overwrite the same value). Fundamentally, you are doing: if (something) value = true; if (otherthing) value = false; I think we don't care about (don't want to change) a pre-existing "value = true" and so we actually just need: if (something && !otherthing) value = true; Or am I missing something? If not, let's actually use the latter and simplify the "something && !otherthing" (e.g. by pulling out variables/functions) until it is readable again. Let me know if you want me to take a stab at that (I promise it won't take weeks again :( ). Comment at: unittests/Format/FormatTest.cpp:3444 + + verif
[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line
djasper added inline comments. Comment at: unittests/Format/FormatTest.cpp:3444 + + verifyFormat("Constructor()\n" + ": (a), b(b) {}", djasper wrote: > I find these tests hard to read and reason about. How about writing them like > this: > > for (int i = 0; i < 4; ++i) { // There might be a better way to iterate > // Test all combinations of parameters that should not have an effect. > Style.AllowAllParametersOfDeclarationOnNextLine = i & 1; > Style.AllowAllConstructorInitializersOnNextLine = i & 2; > > Style.AllowAllConstructorInitializersOnNextLine = true; > verifyFormat("SomeClassWithALongName::Constructor(\n" > "int , int b)\n" > ": (a), b(b) {}", > Style); > // ... more tests > > > Style.AllowAllConstructorInitializersOnNextLine = false; > verifyFormat("SomeClassWithALongName::Constructor(\n" > "int , int b)\n" > ": (a)\n" > ", b(b) {}", > Style); > // ... more tests > } Err.. The second line inside the for-loop was meant to read: Style.AllowAllArgumentsOnNextLine = i & 2; https://reviews.llvm.org/D40988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part
lebedev.ri updated this revision to Diff 155149. lebedev.ri marked 7 inline comments as done. lebedev.ri added a comment. Address @vsk's review notes. - Maintain the stack of currently-being-visited `CastExpr`'s - Use that stack to check whether we are in a `ExplicitCastExpr` - Move logic for deciding whether to emit the check out of `EmitScalarConversion()` - Condense all overloads of `EmitScalarConversion()` down to one. Repository: rC Clang https://reviews.llvm.org/D48958 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanitizer.rst include/clang/Basic/Sanitizers.def include/clang/Basic/Sanitizers.h lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Driver/SanitizerArgs.cpp lib/Driver/ToolChain.cpp test/CodeGen/catch-implicit-integer-truncations.c test/CodeGenCXX/catch-implicit-integer-truncations.cpp test/Driver/fsanitize.c Index: test/Driver/fsanitize.c === --- test/Driver/fsanitize.c +++ test/Driver/fsanitize.c @@ -31,6 +31,21 @@ // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope" // CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent),?){5}"}} +// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-cast %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-IMPLICIT-CAST,CHECK-IMPLICIT-CAST-RECOVER +// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-cast -fsanitize-recover=implicit-cast %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-IMPLICIT-CAST,CHECK-IMPLICIT-CAST-RECOVER +// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-cast -fno-sanitize-recover=implicit-cast %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-IMPLICIT-CAST,CHECK-IMPLICIT-CAST-NORECOVER +// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-cast -fsanitize-trap=implicit-cast %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-IMPLICIT-CAST,CHECK-IMPLICIT-CAST-TRAP +// CHECK-IMPLICIT-CAST: "-fsanitize={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} // ??? +// CHECK-IMPLICIT-CAST-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-TRAP: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}} +// CHECK-IMPLICIT-CAST-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} + // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS // CHECK-BOUNDS: "-fsanitize={{((array-bounds|local-bounds),?){2}"}} Index: test/CodeGenCXX/catch-implicit-integer-truncations.cpp === --- /dev/null +++ test/CodeGenCXX/catch-implicit-integer-truncations.cpp @@ -0,0 +1,230 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK +// RUN: %clang_cc1 -fsanitize=implicit-integer-truncation -fno-sanitize-recover=implicit-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER +// RUN: %clang_cc1 -fsanitize=implicit-integer-truncation -fsanitize-recover=implicit-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER +// RUN: %clang_cc1 -fsanitize=implicit-integer-truncation -fsanitize-trap=implicit-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP + +extern "C" { // Disable name mangling. + +// == // +// Check that explicit cast does not interfere with implicit cast +// == // +// These contain one implicit truncating cast, and one explicit truncating cast. +// We want to make sure that we still diagnose the implicit cast. + +// Implicit truncation after explicit truncation. +// CHECK-LABEL: @explicit_cast_interference0 +unsigned char explicit_cast_interference0(unsigned int c) { + // CHECK-SANITIZE: %[[ANYEXT:.*]] = zext i8 %[[DST:.*]] to i16, !nosanitize + // CHECK-SANITIZE: call
[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part
lebedev.ri added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1694 // handle things like function to ptr-to-function decay etc. Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { Expr *E = CE->getSubExpr(); vsk wrote: > lebedev.ri wrote: > > vsk wrote: > > > lebedev.ri wrote: > > > > vsk wrote: > > > > > I think maintaining a stack of visited cast exprs in the emitter be > > > > > cheaper/simpler than using ASTContext::getParents. You could push CE > > > > > here and use a RAII helper to pop it. The 'isCastPartOfExplicitCast' > > > > > check then simplifies to a quick stack traversal. > > > > Hmm, two things come to mind: > > > > 1. This pessimizes the (most popular) case when the sanitizer is > > > > disabled. > > > > 2. `ASTContext::getParents()` may return more than one parent. I'm not > > > > sure if that matters here? > > > > I'll take a look.. > > > As for (1), it's not necessary to push/pop the stack when this sanitizer > > > is disabled. And for (2), IIUC the only interesting case is > > > "explicit-cast +", and none of the implicit casts here > > > have more than one parent. > > > I think maintaining a stack of visited cast exprs in the emitter be > > > cheaper/simpler than using ASTContext::getParents. > > > > {F6660377} > > > > So yeah, this could work. > > Except sadly it breaks in subtle cases like https://godbolt.org/g/5V2czU > > I have added those tests beforehand. > > > > Is `ASTContext::getParents()` really horribly slow so we want to > > duplicate/maintain/track > > the current AST stack ourselves? > > If so, we will need to maintain the *entire* stack, not just `CastExpr''s... > I think the scan in 'IsTopCastPartOfExplicitCast' can be fixed: while > traversing backwards, you'd need to check that the previously-visited cast > expr is the child of the current expr. That should address the false negative > you pointed out in interference1. I don't yet see what the issue is with > interference0. > > Could you explain why maintaining a stack of unfinished casts wouldn't work? > I don't understand why you'd need the entire stack. My sense is that it's not > required to match the "explicit-cast +" pattern, but I could > easily be missing something here. As for why this might be worth looking > into, I think scanning for an explicit cast is much easier to understand when > working with a stack. > > + @klimek to comment on what to expect in terms of the overhead of > ASTContext::getParents. Regardless of what approach we pick, it would help to > see pre/post-patch compile times for a stage2 build of something like clang > or llc. > I think the scan in 'IsTopCastPartOfExplicitCast' can be fixed: while > traversing backwards, you'd need to check that the previously-visited cast > expr is the child of the current expr. That should address the false negative > you pointed out in interference1. Oh right. That seems to fix the issues. > Could you explain why maintaining a stack of unfinished casts wouldn't work? I didn't think it wouldn't work. I just missed the tidbit about checking children. Then it works. Repository: rC Clang https://reviews.llvm.org/D48958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49083: [HIP] Register/unregister device fat binary only once
yaxunl added a comment. In https://reviews.llvm.org/D49083#1157586, @yaxunl wrote: > In https://reviews.llvm.org/D49083#1157568, @tra wrote: > > > > HIP generates one fat binary for all devices after linking. However, for > > > each compilation > > > unit a ctor function is emitted which register the same fat binary. > > > Measures need to be taken to make sure the fat binary is only registered > > > once. > > > > Are you saying that for HIP there's only one fatbin file with GPU code for > > the complete host executable, even if it consists of multiple HIP TUs? > > > By 'TU' do you mean 'target unit'? > > For HIP there is only one fatbin file with GPU code for the complete host > executable even if there are mulitple GPU sub-targets. Device code for > different sub-targets are bundled together by clang-offload-bundler as one > fatbin. Runtime will extract device code for different sub-targets. It there are multiple translation units, there will be only one fatbin after linking, since the LLVM modules from different translation units will be linked together to generate one fatbin. https://reviews.llvm.org/D49083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49224: [clangd] log request/response messages with method/ID/error at INFO level
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clangd/JSONRPCDispatcher.cpp:138 }); + auto ID = 1; + log("--> {0}({1})", Method, ID); nit I'd suggest putting this statement immediately after the above `FIXME`, it took me a while to understand why it is `1`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo
hokein added a comment. In https://reviews.llvm.org/D49158#1158327, @JonasToth wrote: > Is there a way to add a test, that would trigger the old segfault and show > that it does not happen anymore with this fix? +1, we should have a minimal test case for this fix, https://bugs.llvm.org/show_bug.cgi?id=36150 provides a case, but we need to reduce it (getting rid of the STD header). https://reviews.llvm.org/D49158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
This revision was automatically updated to reflect the committed changes. Closed by commit rL336898: [ASTImporter] Fix infinite recursion on function import with struct definition… (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47946?vs=154991&id=155151#toc Repository: rL LLVM https://reviews.llvm.org/D47946 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -1138,8 +1138,26 @@ DeclarationName &Name, NamedDecl *&ToD, SourceLocation &Loc) { + // Check if RecordDecl is in FunctionDecl parameters to avoid infinite loop. + // example: int struct_in_proto(struct data_t{int a;int b;} *d); + DeclContext *OrigDC = D->getDeclContext(); + FunctionDecl *FunDecl; + if (isa(D) && (FunDecl = dyn_cast(OrigDC)) && + FunDecl->hasBody()) { +SourceRange RecR = D->getSourceRange(); +SourceRange BodyR = FunDecl->getBody()->getSourceRange(); +// If RecordDecl is not in Body (it is a param), we bail out. +if (RecR.isValid() && BodyR.isValid() && +(RecR.getBegin() < BodyR.getBegin() || + BodyR.getEnd() < RecR.getEnd())) { + Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node) + << D->getDeclKindName(); + return true; +} + } + // Import the context of this declaration. - DC = Importer.ImportContext(D->getDeclContext()); + DC = Importer.ImportContext(OrigDC); if (!DC) return true; Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -301,14 +301,25 @@ Unit->enableSourceFileDiagnostics(); } -Decl *import(ASTUnit *ToAST, Decl *FromDecl) { +void lazyInitImporter(ASTUnit *ToAST) { assert(ToAST); if (!Importer) { Importer.reset(new ASTImporter( ToAST->getASTContext(), ToAST->getFileManager(), Unit->getASTContext(), Unit->getFileManager(), false)); } + assert(&ToAST->getASTContext() == &Importer->getToContext()); + createVirtualFileIfNeeded(ToAST, FileName, Code); +} + +Decl *import(ASTUnit *ToAST, Decl *FromDecl) { + lazyInitImporter(ToAST); return Importer->Import(FromDecl); + } + +QualType import(ASTUnit *ToAST, QualType FromType) { + lazyInitImporter(ToAST); + return Importer->Import(FromType); } }; @@ -321,6 +332,26 @@ // vector is expanding, with the list we won't have these issues. std::list FromTUs; + void lazyInitToAST(Language ToLang) { +if (ToAST) + return; +ArgVector ToArgs = getArgVectorForLanguage(ToLang); +// Build the AST from an empty file. +ToAST = tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc"); +ToAST->enableSourceFileDiagnostics(); + } + + TU *findFromTU(Decl *From) { +// Create a virtual file in the To Ctx which corresponds to the file from +// which we want to import the `From` Decl. Without this source locations +// will be invalid in the ToCtx. +auto It = std::find_if(FromTUs.begin(), FromTUs.end(), [From](const TU &E) { + return E.TUDecl == From->getTranslationUnitDecl(); +}); +assert(It != FromTUs.end()); +return &*It; + } + public: // We may have several From context but only one To context. std::unique_ptr ToAST; @@ -394,26 +425,17 @@ // May be called several times in a given test. // The different instances of the param From may have different ASTContext. Decl *Import(Decl *From, Language ToLang) { -if (!ToAST) { - ArgVector ToArgs = getArgVectorForLanguage(ToLang); - // Build the AST from an empty file. - ToAST = - tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc"); - ToAST->enableSourceFileDiagnostics(); -} - -// Create a virtual file in the To Ctx which corresponds to the file from -// which we want to import the `From` Decl. Without this source locations -// will be invalid in the ToCtx. -auto It = std::find_if(FromTUs.begin(), FromTUs.end(), [From](const TU &E) { - return E.TUDecl == From->getTranslationUnitDecl(); -}); -assert(It != FromTUs.end()); -createVirtualFileIfNeeded(ToAST.get(), It->FileName, It->Code); - -return It->import(ToAST.get(), From); +lazyInitToAST(ToLang); +TU *FromTU = findFromTU(From); +return FromTU->import(ToAST.get(), From); } + QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang) { +lazyInitToAST(ToLang); +TU *FromTU = find
r336898 - [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
Author: martong Date: Thu Jul 12 04:50:21 2018 New Revision: 336898 URL: http://llvm.org/viewvc/llvm-project?rev=336898&view=rev Log: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters Summary: Importing a function having a struct definition in the parameter list causes a crash in the importer via infinite recursion. This patch avoids the crash and reports such functions as not supported. Unit tests make sure that normal struct definitions inside function bodies work normally on the other hand and LLDB-like type imports also do. Reviewers: a.sidorin, martong Differential Revision: https://reviews.llvm.org/D47946 Patch by Zoltan Gera! Modified: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/unittests/AST/ASTImporterTest.cpp Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=336898&r1=336897&r2=336898&view=diff == --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Thu Jul 12 04:50:21 2018 @@ -1138,8 +1138,26 @@ bool ASTNodeImporter::ImportDeclParts(Na DeclarationName &Name, NamedDecl *&ToD, SourceLocation &Loc) { + // Check if RecordDecl is in FunctionDecl parameters to avoid infinite loop. + // example: int struct_in_proto(struct data_t{int a;int b;} *d); + DeclContext *OrigDC = D->getDeclContext(); + FunctionDecl *FunDecl; + if (isa(D) && (FunDecl = dyn_cast(OrigDC)) && + FunDecl->hasBody()) { +SourceRange RecR = D->getSourceRange(); +SourceRange BodyR = FunDecl->getBody()->getSourceRange(); +// If RecordDecl is not in Body (it is a param), we bail out. +if (RecR.isValid() && BodyR.isValid() && +(RecR.getBegin() < BodyR.getBegin() || + BodyR.getEnd() < RecR.getEnd())) { + Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node) + << D->getDeclKindName(); + return true; +} + } + // Import the context of this declaration. - DC = Importer.ImportContext(D->getDeclContext()); + DC = Importer.ImportContext(OrigDC); if (!DC) return true; Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=336898&r1=336897&r2=336898&view=diff == --- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original) +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Thu Jul 12 04:50:21 2018 @@ -301,14 +301,25 @@ class ASTImporterTestBase : public Param Unit->enableSourceFileDiagnostics(); } -Decl *import(ASTUnit *ToAST, Decl *FromDecl) { +void lazyInitImporter(ASTUnit *ToAST) { assert(ToAST); if (!Importer) { Importer.reset(new ASTImporter( ToAST->getASTContext(), ToAST->getFileManager(), Unit->getASTContext(), Unit->getFileManager(), false)); } + assert(&ToAST->getASTContext() == &Importer->getToContext()); + createVirtualFileIfNeeded(ToAST, FileName, Code); +} + +Decl *import(ASTUnit *ToAST, Decl *FromDecl) { + lazyInitImporter(ToAST); return Importer->Import(FromDecl); + } + +QualType import(ASTUnit *ToAST, QualType FromType) { + lazyInitImporter(ToAST); + return Importer->Import(FromType); } }; @@ -321,6 +332,26 @@ class ASTImporterTestBase : public Param // vector is expanding, with the list we won't have these issues. std::list FromTUs; + void lazyInitToAST(Language ToLang) { +if (ToAST) + return; +ArgVector ToArgs = getArgVectorForLanguage(ToLang); +// Build the AST from an empty file. +ToAST = tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc"); +ToAST->enableSourceFileDiagnostics(); + } + + TU *findFromTU(Decl *From) { +// Create a virtual file in the To Ctx which corresponds to the file from +// which we want to import the `From` Decl. Without this source locations +// will be invalid in the ToCtx. +auto It = std::find_if(FromTUs.begin(), FromTUs.end(), [From](const TU &E) { + return E.TUDecl == From->getTranslationUnitDecl(); +}); +assert(It != FromTUs.end()); +return &*It; + } + public: // We may have several From context but only one To context. std::unique_ptr ToAST; @@ -394,26 +425,17 @@ public: // May be called several times in a given test. // The different instances of the param From may have different ASTContext. Decl *Import(Decl *From, Language ToLang) { -if (!ToAST) { - ArgVector ToArgs = getArgVectorForLanguage(ToLang); - // Build the AST from an empty file. - ToAST = - tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc"); - ToAST->ena
[clang-tools-extra] r336899 - [clangd] log request/response messages with method/ID/error at INFO level
Author: sammccall Date: Thu Jul 12 04:52:18 2018 New Revision: 336899 URL: http://llvm.org/viewvc/llvm-project?rev=336899&view=rev Log: [clangd] log request/response messages with method/ID/error at INFO level Summary: Bodies are logged at VERBOSE level (since r336785), tweak the formatting. Reviewers: hokein Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D49224 Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp?rev=336899&r1=336898&r2=336899&view=diff == --- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp (original) +++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Thu Jul 12 04:52:18 2018 @@ -69,7 +69,7 @@ void JSONOutput::writeMessage(const json Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S; Outs.flush(); } - vlog("--> {0}\n", S); + vlog(">>> {0}\n", S); } void JSONOutput::log(Logger::Level Level, @@ -99,6 +99,7 @@ void clangd::reply(json::Value &&Result) return; } RequestSpan::attach([&](json::Object &Args) { Args["Reply"] = Result; }); + log("--> reply({0})", *ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -116,6 +117,7 @@ void clangd::replyError(ErrorCode code, }); if (auto ID = Context::current().get(RequestID)) { +log("--> reply({0}) error: {1}", *ID, Message); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -128,16 +130,18 @@ void clangd::replyError(ErrorCode code, } void clangd::call(StringRef Method, json::Value &&Params) { - // FIXME: Generate/Increment IDs for every request so that we can get proper - // replies once we need to. RequestSpan::attach([&](json::Object &Args) { Args["Call"] = json::Object{{"method", Method.str()}, {"params", Params}}; }); + // FIXME: Generate/Increment IDs for every request so that we can get proper + // replies once we need to. + auto ID = 1; + log("--> {0}({1})", Method, ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ {"jsonrpc", "2.0"}, - {"id", 1}, + {"id", ID}, {"method", Method}, {"params", std::move(Params)}, }); @@ -148,6 +152,23 @@ void JSONRPCDispatcher::registerHandler( Handlers[Method] = std::move(H); } +static void logIncomingMessage(const llvm::Optional &ID, + llvm::Optional Method, + const json::Object *Error) { + if (Method) { // incoming request +if (ID) // call + log("<-- {0}({1})", *Method, *ID); +else // notification + log("<-- {0}", *Method); + } else if (ID) { // response, ID must be provided +if (Error) + log("<-- reply({0}) error: {1}", *ID, + Error->getString("message").getValueOr("")); +else + log("<-- reply({0})", *ID); + } +} + bool JSONRPCDispatcher::call(const json::Value &Message, JSONOutput &Out) const { // Message must be an object with "jsonrpc":"2.0". @@ -158,9 +179,9 @@ bool JSONRPCDispatcher::call(const json: llvm::Optional ID; if (auto *I = Object->get("id")) ID = std::move(*I); - // Method must be given. auto Method = Object->getString("method"); - if (!Method) + logIncomingMessage(ID, Method, Object->getObject("error")); + if (!Method) // We only handle incoming requests, and ignore responses. return false; // Params should be given, use null if not. json::Value Params = nullptr; @@ -333,13 +354,13 @@ void clangd::runLanguageServerLoop(std:: if (auto JSON = ReadMessage(In, Out)) { if (auto Doc = json::parse(*JSON)) { // Log the formatted message. -vlog(Out.Pretty ? "<-- {0:2}\n" : "<-- {0}\n", *Doc); +vlog(Out.Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc); // Finally, execute the action for this JSON message. if (!Dispatcher.call(*Doc, Out)) elog("JSON dispatch failed!"); } else { // Parse error. Log the raw message. -vlog("<-- {0}\n", *JSON); +vlog("<<< {0}\n", *JSON); elog("JSON parse error: {0}", llvm::toString(Doc.takeError())); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49224: [clangd] log request/response messages with method/ID/error at INFO level
This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rL336899: [clangd] log request/response messages with method/ID/error at INFO level (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49224?vs=155132&id=155152#toc Repository: rL LLVM https://reviews.llvm.org/D49224 Files: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Index: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp === --- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp +++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp @@ -69,7 +69,7 @@ Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S; Outs.flush(); } - vlog("--> {0}\n", S); + vlog(">>> {0}\n", S); } void JSONOutput::log(Logger::Level Level, @@ -99,6 +99,7 @@ return; } RequestSpan::attach([&](json::Object &Args) { Args["Reply"] = Result; }); + log("--> reply({0})", *ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -116,6 +117,7 @@ }); if (auto ID = Context::current().get(RequestID)) { +log("--> reply({0}) error: {1}", *ID, Message); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ @@ -128,16 +130,18 @@ } void clangd::call(StringRef Method, json::Value &&Params) { - // FIXME: Generate/Increment IDs for every request so that we can get proper - // replies once we need to. RequestSpan::attach([&](json::Object &Args) { Args["Call"] = json::Object{{"method", Method.str()}, {"params", Params}}; }); + // FIXME: Generate/Increment IDs for every request so that we can get proper + // replies once we need to. + auto ID = 1; + log("--> {0}({1})", Method, ID); Context::current() .getExisting(RequestOut) ->writeMessage(json::Object{ {"jsonrpc", "2.0"}, - {"id", 1}, + {"id", ID}, {"method", Method}, {"params", std::move(Params)}, }); @@ -148,6 +152,23 @@ Handlers[Method] = std::move(H); } +static void logIncomingMessage(const llvm::Optional &ID, + llvm::Optional Method, + const json::Object *Error) { + if (Method) { // incoming request +if (ID) // call + log("<-- {0}({1})", *Method, *ID); +else // notification + log("<-- {0}", *Method); + } else if (ID) { // response, ID must be provided +if (Error) + log("<-- reply({0}) error: {1}", *ID, + Error->getString("message").getValueOr("")); +else + log("<-- reply({0})", *ID); + } +} + bool JSONRPCDispatcher::call(const json::Value &Message, JSONOutput &Out) const { // Message must be an object with "jsonrpc":"2.0". @@ -158,9 +179,9 @@ llvm::Optional ID; if (auto *I = Object->get("id")) ID = std::move(*I); - // Method must be given. auto Method = Object->getString("method"); - if (!Method) + logIncomingMessage(ID, Method, Object->getObject("error")); + if (!Method) // We only handle incoming requests, and ignore responses. return false; // Params should be given, use null if not. json::Value Params = nullptr; @@ -333,13 +354,13 @@ if (auto JSON = ReadMessage(In, Out)) { if (auto Doc = json::parse(*JSON)) { // Log the formatted message. -vlog(Out.Pretty ? "<-- {0:2}\n" : "<-- {0}\n", *Doc); +vlog(Out.Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc); // Finally, execute the action for this JSON message. if (!Dispatcher.call(*Doc, Out)) elog("JSON dispatch failed!"); } else { // Parse error. Log the raw message. -vlog("<-- {0}\n", *JSON); +vlog("<<< {0}\n", *JSON); elog("JSON parse error: {0}", llvm::toString(Doc.takeError())); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49213: [analyzer] pr38072: Suppress an assertion failure for eliding the same destructor twice due to the default argument problem.
alexfh added a comment. FTR, this is http://llvm.org/PR38072 Repository: rC Clang https://reviews.llvm.org/D49213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49228: [analyzer][UninitializedObjectChecker] Void pointer objects are casted back to their dynmic type in note message
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, whisperity. Repository: rC Clang https://reviews.llvm.org/D49228 Files: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp === --- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp +++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp @@ -287,7 +287,7 @@ } struct IntDynTypedVoidPointerTest1 { - void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}} + void *vptr; // expected-note{{uninitialized pointee 'this->static_cast(vptr)'}} int dontGetFilteredByNonPedanticMode = 0; IntDynTypedVoidPointerTest1(void *vptr) : vptr(vptr) {} // expected-warning{{1 uninitialized field}} @@ -300,8 +300,8 @@ struct RecordDynTypedVoidPointerTest { struct RecordType { -int x; // expected-note{{uninitialized field 'this->vptr->x'}} -int y; // expected-note{{uninitialized field 'this->vptr->y'}} +int x; // expected-note{{uninitialized field 'this->static_cast(vptr)->x'}} +int y; // expected-note{{uninitialized field 'this->static_cast(vptr)->y'}} }; void *vptr; @@ -317,9 +317,9 @@ struct NestedNonVoidDynTypedVoidPointerTest { struct RecordType { -int x; // expected-note{{uninitialized field 'this->vptr->x'}} -int y; // expected-note{{uninitialized field 'this->vptr->y'}} -void *vptr; // expected-note{{uninitialized pointee 'this->vptr->vptr'}} +int x; // expected-note{{uninitialized field 'this->static_cast(vptr)->x'}} +int y; // expected-note{{uninitialized field 'this->static_cast(vptr)->y'}} +void *vptr; // expected-note{{uninitialized pointee 'this->static_cast(vptr)->static_cast(vptr)'}} }; void *vptr; Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -50,31 +50,60 @@ void checkEndFunction(CheckerContext &C) const; }; -llvm::ImmutableListFactory Factory; - /// Represents a field chain. A field chain is a vector of fields where the /// first element of the chain is the object under checking (not stored), and /// every other element is a field, and the element that precedes it is the /// object that contains it. /// /// Note that this class is immutable, and new fields may only be added through /// constructor calls. class FieldChainInfo { + using FieldChainImpl = llvm::ImmutableListImpl; + + // If the fieldchain has void pointers or nonloc::LocAsPointer objects, + // they have to be casted back in order to dereference them. + // + // struct B { + // void *ptr; + // B(void *ptr) : ptr(ptr) {} + // }; + // struct A { int x; } a; // say this isn't zero initialized + // + // B b(&a); // warning + // + // For cases like this, we can't report that 'this->ptr->a' is uninitialized, + // since void pointers can't be dereferenced, but rather the note message + // should say 'static_cast(this->ptr)->a' is uninitialized. + // + // We'll store which elements of the fieldchain needs to be casted back, + // and the type it needs to be casted to. + using CastBack = const std::pair; + using CastBacksListImpl = llvm::ImmutableListImpl; + +public: + // We'll expose these types for a nicer looking factory instantionation. using FieldChain = llvm::ImmutableList; + using CastBackList = llvm::ImmutableList; +private: FieldChain Chain; + CastBackList CastBacks; const bool IsDereferenced = false; public: FieldChainInfo() = default; FieldChainInfo(const FieldChainInfo &Other, const bool IsDereferenced) - : Chain(Other.Chain), IsDereferenced(IsDereferenced) {} + : Chain(Other.Chain), CastBacks(Other.CastBacks), +IsDereferenced(IsDereferenced) {} FieldChainInfo(const FieldChainInfo &Other, const FieldRegion *FR, const bool IsDereferenced = false); + FieldChainInfo(const FieldChainInfo &Other, const FieldRegion *FR, + const bool IsDereferenced, QualType CastBackType); + bool contains(const FieldRegion *FR) const { return Chain.contains(FR); } bool isPointer() const; @@ -90,8 +119,8 @@ /// elements in reverse order, and have no reverse iterators, we use a /// recursive function to print the fieldchain correctly. The last element in /// the chain is to be printed by `print`. - static void printTail(llvm::raw_ostream &Out, -const llvm::ImmutableListImpl *L); + static void printTail(llvm::raw_ostream &Out, const FieldChainImpl *L, +const CastBacksListImpl *C); friend s
[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile
simark added a comment. In https://reviews.llvm.org/D49197#1159704, @ioeric wrote: > In https://reviews.llvm.org/D49197#1158972, @simark wrote: > > > Background: I'm trying to fix the cases why we receive a FileEntry without > > a real path computed in clangd, so we can avoid having to compute it > > ourselves. > > > I'm curious how you use `getVirtualFile` in your clangd tests. In general, > I'd highly recommend virtual file system in favor of remapped files for > clangd tests. I don't use getVirtualFile directly. I just use `ClangdServer` and look at the paths it outputs. It just happens that for some file, clang first opened it internally using `getVirtualFile` before using `getFile`, so the real path ends up missing`. Patch https://reviews.llvm.org/D48687 adds code to compensate that, but I added some logging to know when it happens, so we can try to fix it at the root (ideally, clang could always give us the real path so we don't have to compute it ourselves): ./tools/clang/tools/extra/unittests/clangd/ClangdTests --gtest_filter='*RelPathsInCompileCommand*' Note: Google Test filter = *RelPathsInCompileCommand* [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from GoToDefinition [ RUN ] GoToDefinition.RelPathsInCompileCommand Updating file /clangd-test/src/foo.cpp with command [/clangd-test/build] clang -ffreestanding ../src/foo.cpp -resource-dir=/home/emaisin/build/llvm/tools/clang/tools/extra/unittests/clangd/../lib/clang/7.0.0 Preamble for file /clangd-test/src/foo.cpp cannot be reused. Attempting to rebuild it. Built preamble of size 171284 for file /clangd-test/src/foo.cpp FileEntry for ../src/foo.cpp did not contain the real path. < HERE FileEntry for /clangd-test/src/header_in_preamble.h did not contain the real path. < AND HERE [ OK ] GoToDefinition.RelPathsInCompileCommand (42 ms) [--] 1 test from GoToDefinition (42 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (43 ms total) [ PASSED ] 1 test. I'm investigating why clang failed to provide that to us in these two cases. Repository: rC Clang https://reviews.llvm.org/D49197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336901 - [analyzer][UninitializedObjectChecker] Moved non-member functions out of the anonymous namespace
Author: szelethus Date: Thu Jul 12 06:13:46 2018 New Revision: 336901 URL: http://llvm.org/viewvc/llvm-project?rev=336901&view=rev Log: [analyzer][UninitializedObjectChecker] Moved non-member functions out of the anonymous namespace As the code for the checker grew, it became increasinly difficult to see whether a function was global or statically defined. In this patch, anything that isn't a type declaration or definition was moved out of the anonymous namespace and is marked as static. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp?rev=336901&r1=336900&r2=336901&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp Thu Jul 12 06:13:46 2018 @@ -50,8 +50,6 @@ public: void checkEndFunction(CheckerContext &C) const; }; -llvm::ImmutableListFactory Factory; - /// Represents a field chain. A field chain is a vector of fields where the /// first element of the chain is the object under checking (not stored), and /// every other element is a field, and the element that precedes it is the @@ -205,32 +203,37 @@ private: // TODO: Add a support for nonloc::LocAsInteger. }; +} // end of anonymous namespace + +// Static variable instantionations. + +static llvm::ImmutableListFactory Factory; + // Utility function declarations. /// Returns the object that was constructed by CtorDecl, or None if that isn't /// possible. -Optional +static Optional getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context); /// Checks whether the constructor under checking is called by another /// constructor. -bool isCalledByConstructor(const CheckerContext &Context); +static bool isCalledByConstructor(const CheckerContext &Context); /// Returns whether FD can be (transitively) dereferenced to a void pointer type /// (void*, void**, ...). The type of the region behind a void pointer isn't /// known, and thus FD can not be analyzed. -bool isVoidPointer(const FieldDecl *FD); +static bool isVoidPointer(const FieldDecl *FD); /// Returns true if T is a primitive type. We'll call a type primitive if it's /// either a BuiltinType or an EnumeralType. -bool isPrimitiveType(const QualType &T) { +static bool isPrimitiveType(const QualType &T) { return T->isBuiltinType() || T->isEnumeralType(); } /// Constructs a note message for a given FieldChainInfo object. -void printNoteMessage(llvm::raw_ostream &Out, const FieldChainInfo &Chain); - -} // end of anonymous namespace +static void printNoteMessage(llvm::raw_ostream &Out, + const FieldChainInfo &Chain); //===--===// // Methods for UninitializedObjectChecker. @@ -638,9 +641,7 @@ void FieldChainInfo::printTail( // Utility functions. //===--===// -namespace { - -bool isVoidPointer(const FieldDecl *FD) { +static bool isVoidPointer(const FieldDecl *FD) { QualType T = FD->getType(); while (!T.isNull()) { @@ -651,7 +652,7 @@ bool isVoidPointer(const FieldDecl *FD) return false; } -Optional +static Optional getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context) { Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(), @@ -668,7 +669,7 @@ getObjectVal(const CXXConstructorDecl *C // TODO: We should also check that if the constructor was called by another // constructor, whether those two are in any relation to one another. In it's // current state, this introduces some false negatives. -bool isCalledByConstructor(const CheckerContext &Context) { +static bool isCalledByConstructor(const CheckerContext &Context) { const LocationContext *LC = Context.getLocationContext()->getParent(); while (LC) { @@ -680,7 +681,8 @@ bool isCalledByConstructor(const Checker return false; } -void printNoteMessage(llvm::raw_ostream &Out, const FieldChainInfo &Chain) { +static void printNoteMessage(llvm::raw_ostream &Out, + const FieldChainInfo &Chain) { if (Chain.isPointer()) { if (Chain.isDereferenced()) Out << "uninitialized pointee 'this->"; @@ -692,8 +694,6 @@ void printNoteMessage(llvm::raw_ostream Out << "'"; } -} // end of anonymous namespace - void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) { auto Chk = Mgr.registerChecker(); Chk->IsPedantic = Mgr.getAnalyzerOptions().getBooleanOption( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:
[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile
simark added inline comments. Comment at: lib/Basic/FileManager.cpp:395 + SmallString<128> RealPathName; + if (!FS->getRealPath(InterndFileName, RealPathName)) +UFE->RealPathName = RealPathName.str(); ioeric wrote: > simark wrote: > > ioeric wrote: > > > It seems that at this point, we have failed to find `FileName` in vfs > > > (with `getStatValue` above), so `getRealPath` would also fail? > > > > > > In general, the virtual file here can be an actual *virtual* file that > > > doesn't exist anywhere, and I think there are users who use this to map > > > virtual file (possibly with relative paths) into file manager (while they > > > should really use overlay vfs!). > > > It seems that at this point, we have failed to find FileName in vfs (with > > > getStatValue above), so getRealPath would also fail? > > > > From what I understood, this code will be executed if the file actually > > exists but it's the first time we access it (we won't take the return at > > line 373). If we take the return at line 373, then some previous > > invocation of getFile or getVirtualFile has created the file, and that > > invocation would have been responsible for computing the real path. > > > > > In general, the virtual file here can be an actual *virtual* file that > > > doesn't exist anywhere, and I think there are users who use this to map > > > virtual file (possibly with relative paths) into file manager (while they > > > should really use overlay vfs!). > > > > My thinking was that the worst that could happen is that `getRealPath` > > fails in that case. > > From what I understood, this code will be executed if the file actually > > exists but it's the first time we access it (we won't take the return at > > line 373). If we take the return at line 373, then some previous invocation > > of getFile or getVirtualFile has created the file, and that invocation > > would have been responsible for computing the real path. > I see. Thanks for the explanation! > > My thinking was that the worst that could happen is that getRealPath fails > > in that case. > It might make sense to take a look at how `getVirtualFile` is used in clang. > For example, in CompilerInstance, it's used to create virtual FileEntry for > remapped files > (https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CompilerInstance.cpp#L329), > and contents will be overwritten in `SourceManager`. This usage is arguable > as we have overlay file system nowadays. But in this case, if we try to > `getRealPath` on a remapped file, it might happen that we accidentally get an > absolute path on the real file system, if there happens to be a file with the > same path (relative to the CWD) on disk. This can be surprising and could be > hard to debug. This is not unusual as people tend to use file remapping to > overwrite the content of disk files with dirty buffers. > > In general, virtual files in FileManager and virtual files in vfs are > different things, and mixing them up further would likely cause more > confusion in the future. We should really get rid of the remapped virtual > files in `FileManager` and implement them properly with an overlay fs... Ok, I'm not familiar with what file remapping is. An alternative approach would be to compute the real path in `getFile` when there is a cache hit but the file entry previously represented a virtual file. Eessentially, we would "upgrade" the virtual file to a standard one. Repository: rC Clang https://reviews.llvm.org/D49197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a comment. In https://reviews.llvm.org/D48903#1159303, @ioeric wrote: > For example, suppose you have header search directories `-I/path/to/include` > and `-I.` in your compile command. When preprocessor searches for an #include > "x.h", it will try to stat "/path/to/include/x.h" and "./x.h" and take the > first one that exists. This can introduce indeterminism for the path (./x.h > or /abs/x.h) you later get for the header file, e.g. when you try to look up > file name by `FileID` through `SourceManager`. The path you get for a > `FileEntry` or `FileID` would depend on how clang looks up a file and how a > file is first opened into `SourceManager`/`FileManager`. It seems that the > current behavior of clangd + in-memory file system would give you paths that > are relative to the working directory for both cases. I'm not sure if that's > the best behavior, but I think the consistency has its value. For example, in > unit tests where in-memory file systems are heavily used, it's important to > have a way to compare the reported file path (e.g. file path corresponding to > a source location) with the expected paths. We could choose to always return > real path, which could be potentially expensive, or we could require users to > always compare real paths (it's unclear how well this would work though e.g. > ClangTool doesn't expose its vfs), but they need to be enforced by the API. > Otherwise, I worry it would cause more confusions for folks who use clang > with in-memory file system in the future. It's hard to tell without seeing an actual failing use case. I understand the argument, but I think the necessity to work as closely as `RealFileSystem` as possible is important. Otherwise, it becomes impossible to reproduce bugs happening in the real world in unittests. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49235: [ASTImporter] Import described template (if any) of function.
balazske created this revision. Herald added subscribers: cfe-commits, martong. Herald added a reviewer: a.sidorin. When a function is imported, check if it has a described template. The name lookup is corrected to find the templated entity in this case. The described template of the function is imported too. Repository: rC Clang https://reviews.llvm.org/D49235 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1125,7 +1125,7 @@ } TEST_P(ASTImporterTestBase, - DISABLED_ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) { + ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) { Decl *FromTU = getTuDecl("template void f(){}", Lang_CXX); auto FromFT = FirstDeclMatcher().match( FromTU, functionTemplateDecl()); Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2499,6 +2499,7 @@ return ToD; const FunctionDecl *FoundByLookup = nullptr; + FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate(); // If this is a function template specialization, then try to find the same // existing specialization in the "to" context. The localUncachedLookup @@ -2525,6 +2526,14 @@ if (!FoundDecl->isInIdentifierNamespace(IDNS)) continue; + // If template was found, look at the templated function. + if (FromFT) { +if (auto *Template = dyn_cast(FoundDecl)) + FoundDecl = Template->getTemplatedDecl(); +else + continue; + } + if (auto *FoundFunction = dyn_cast(FoundDecl)) { if (FoundFunction->hasExternalFormalLinkage() && D->hasExternalFormalLinkage()) { @@ -2700,6 +2709,14 @@ ToFunction->setType(T); } + // Import the describing template function, if any. + if (FromFT) { +if (auto *ToFT = dyn_cast(Importer.Import(FromFT))) + ToFunction->setDescribedFunctionTemplate(ToFT); +else + return nullptr; + } + if (D->doesThisDeclarationHaveABody()) { if (Stmt *FromBody = D->getBody()) { if (Stmt *ToBody = Importer.Import(FromBody)) { Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1125,7 +1125,7 @@ } TEST_P(ASTImporterTestBase, - DISABLED_ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) { + ImportOfTemplatedDeclShouldImportTheFunctionTemplateDecl) { Decl *FromTU = getTuDecl("template void f(){}", Lang_CXX); auto FromFT = FirstDeclMatcher().match( FromTU, functionTemplateDecl()); Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2499,6 +2499,7 @@ return ToD; const FunctionDecl *FoundByLookup = nullptr; + FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate(); // If this is a function template specialization, then try to find the same // existing specialization in the "to" context. The localUncachedLookup @@ -2525,6 +2526,14 @@ if (!FoundDecl->isInIdentifierNamespace(IDNS)) continue; + // If template was found, look at the templated function. + if (FromFT) { +if (auto *Template = dyn_cast(FoundDecl)) + FoundDecl = Template->getTemplatedDecl(); +else + continue; + } + if (auto *FoundFunction = dyn_cast(FoundDecl)) { if (FoundFunction->hasExternalFormalLinkage() && D->hasExternalFormalLinkage()) { @@ -2700,6 +2709,14 @@ ToFunction->setType(T); } + // Import the describing template function, if any. + if (FromFT) { +if (auto *ToFT = dyn_cast(Importer.Import(FromFT))) + ToFunction->setDescribedFunctionTemplate(ToFT); +else + return nullptr; + } + if (D->doesThisDeclarationHaveABody()) { if (Stmt *FromBody = D->getBody()) { if (Stmt *ToBody = Importer.Import(FromBody)) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
ioeric added a comment. In https://reviews.llvm.org/D48903#1159985, @simark wrote: > In https://reviews.llvm.org/D48903#1159303, @ioeric wrote: > > > For example, suppose you have header search directories > > `-I/path/to/include` and `-I.` in your compile command. When preprocessor > > searches for an #include "x.h", it will try to stat "/path/to/include/x.h" > > and "./x.h" and take the first one that exists. This can introduce > > indeterminism for the path (./x.h or /abs/x.h) you later get for the header > > file, e.g. when you try to look up file name by `FileID` through > > `SourceManager`. The path you get for a `FileEntry` or `FileID` would > > depend on how clang looks up a file and how a file is first opened into > > `SourceManager`/`FileManager`. It seems that the current behavior of > > clangd + in-memory file system would give you paths that are relative to > > the working directory for both cases. I'm not sure if that's the best > > behavior, but I think the consistency has its value. For example, in unit > > tests where in-memory file systems are heavily used, it's important to have > > a way to compare the reported file path (e.g. file path corresponding to a > > source location) with the expected paths. We could choose to always return > > real path, which could be potentially expensive, or we could require users > > to always compare real paths (it's unclear how well this would work though > > e.g. ClangTool doesn't expose its vfs), but they need to be enforced by the > > API. Otherwise, I worry it would cause more confusions for folks who use > > clang with in-memory file system in the future. > > > It's hard to tell without seeing an actual failing use case. I think the clang-move test you mitigated in https://reviews.llvm.org/D48951 is an example. When setting up compiler instance, it uses `-I.` in the compile command (https://github.com/llvm-mirror/clang-tools-extra/blob/master/unittests/clang-move/ClangMoveTests.cpp#L236), which results in the header paths that start with `./`. If you change replace `.` with the absolute path of the working directory e.g. `-I/usr/include`, the paths you get will start with `/usr/include/`. This is caused by the way preprocessor looks up include headers. We could require users (e.g. the clang-move test writer) to clean up dots, but this has to be enforced somehow by the framework. > I understand the argument, but I think the necessity to work as closely as > `RealFileSystem` as possible is important. Otherwise, it becomes impossible > to reproduce bugs happening in the real world in unittests. FWIW, I don't think we could reproduce all real world bugs with unit tests ;) But I agree with you that we should try to converge the behavior if possible, and I support the motivation of this change ;) This change would be perfectly fine as is if in-mem fs is always used directly by users, but the problem is that it's also used inside clang and clang tooling, where users don't control over how files are opened. My point is we should do this in a way that doesn't introduce inconsistency/confusion for users of clang and tooling framework. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49142: [clangd] Extract FileSystemProvider into a separate header. NFC
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/FSProvider.cpp:15 + +IntrusiveRefCntPtr RealFileSystemProvider::getFileSystem() { + return vfs::getRealFileSystem(); seems like this should just be inlined Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336909 - [clangd] Extract FileSystemProvider into a separate header. NFC
Author: sammccall Date: Thu Jul 12 07:49:52 2018 New Revision: 336909 URL: http://llvm.org/viewvc/llvm-project?rev=336909&view=rev Log: [clangd] Extract FileSystemProvider into a separate header. NFC Reviewers: sammccall Reviewed By: sammccall Subscribers: mgorny, ioeric, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D49142 Added: clang-tools-extra/trunk/clangd/FSProvider.h Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=336909&r1=336908&r2=336909&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Jul 12 07:49:52 2018 @@ -68,10 +68,6 @@ public: } // namespace -IntrusiveRefCntPtr RealFileSystemProvider::getFileSystem() { - return vfs::getRealFileSystem(); -} - ClangdServer::Options ClangdServer::optsForTest() { ClangdServer::Options Opts; Opts.UpdateDebounce = std::chrono::steady_clock::duration::zero(); // Faster! Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=336909&r1=336908&r2=336909&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Jul 12 07:49:52 2018 @@ -12,6 +12,7 @@ #include "ClangdUnit.h" #include "CodeComplete.h" +#include "FSProvider.h" #include "Function.h" #include "GlobalCompilationDatabase.h" #include "Protocol.h" @@ -42,22 +43,6 @@ public: std::vector Diagnostics) = 0; }; -class FileSystemProvider { -public: - virtual ~FileSystemProvider() = default; - /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing. - /// Context::current() will be the context passed to the clang entrypoint, - /// such as addDocument(), and will also be propagated to result callbacks. - /// Embedders may use this to isolate filesystem accesses. - virtual IntrusiveRefCntPtr getFileSystem() = 0; -}; - -class RealFileSystemProvider : public FileSystemProvider { -public: - /// Returns getRealFileSystem(). - IntrusiveRefCntPtr getFileSystem() override; -}; - /// Provides API to manage ASTs for a collection of C++ files and request /// various language features. /// Currently supports async diagnostics, code completion, formatting and goto Added: clang-tools-extra/trunk/clangd/FSProvider.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FSProvider.h?rev=336909&view=auto == --- clang-tools-extra/trunk/clangd/FSProvider.h (added) +++ clang-tools-extra/trunk/clangd/FSProvider.h Thu Jul 12 07:49:52 2018 @@ -0,0 +1,42 @@ +//===--- FSProvider.h - VFS provider for ClangdServer *- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H + +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" + +namespace clang { +namespace clangd { + +// Wrapper for vfs::FileSystem for use in multithreaded programs like clangd. +// As FileSystem is not threadsafe, concurrent threads must each obtain one. +class FileSystemProvider { +public: + virtual ~FileSystemProvider() = default; + /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing. + /// Context::current() will be the context passed to the clang entrypoint, + /// such as addDocument(), and will also be propagated to result callbacks. + /// Embedders may use this to isolate filesystem accesses. + virtual IntrusiveRefCntPtr getFileSystem() = 0; +}; + +class RealFileSystemProvider : public FileSystemProvider { +public: + // FIXME: returns the single real FS instance, which is not threadsafe. + IntrusiveRefCntPtr getFileSystem() override { +return vfs::getRealFileSystem(); + } +}; + +} // namespace clangd +} // namespace clang + +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49142: [clangd] Extract FileSystemProvider into a separate header. NFC
This revision was automatically updated to reflect the committed changes. Closed by commit rL336909: [clangd] Extract FileSystemProvider into a separate header. NFC (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49142?vs=154823&id=155178#toc Repository: rL LLVM https://reviews.llvm.org/D49142 Files: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/FSProvider.h Index: clang-tools-extra/trunk/clangd/FSProvider.h === --- clang-tools-extra/trunk/clangd/FSProvider.h +++ clang-tools-extra/trunk/clangd/FSProvider.h @@ -0,0 +1,42 @@ +//===--- FSProvider.h - VFS provider for ClangdServer *- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H + +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" + +namespace clang { +namespace clangd { + +// Wrapper for vfs::FileSystem for use in multithreaded programs like clangd. +// As FileSystem is not threadsafe, concurrent threads must each obtain one. +class FileSystemProvider { +public: + virtual ~FileSystemProvider() = default; + /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing. + /// Context::current() will be the context passed to the clang entrypoint, + /// such as addDocument(), and will also be propagated to result callbacks. + /// Embedders may use this to isolate filesystem accesses. + virtual IntrusiveRefCntPtr getFileSystem() = 0; +}; + +class RealFileSystemProvider : public FileSystemProvider { +public: + // FIXME: returns the single real FS instance, which is not threadsafe. + IntrusiveRefCntPtr getFileSystem() override { +return vfs::getRealFileSystem(); + } +}; + +} // namespace clangd +} // namespace clang + +#endif Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -68,10 +68,6 @@ } // namespace -IntrusiveRefCntPtr RealFileSystemProvider::getFileSystem() { - return vfs::getRealFileSystem(); -} - ClangdServer::Options ClangdServer::optsForTest() { ClangdServer::Options Opts; Opts.UpdateDebounce = std::chrono::steady_clock::duration::zero(); // Faster! Index: clang-tools-extra/trunk/clangd/ClangdServer.h === --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -12,6 +12,7 @@ #include "ClangdUnit.h" #include "CodeComplete.h" +#include "FSProvider.h" #include "Function.h" #include "GlobalCompilationDatabase.h" #include "Protocol.h" @@ -42,22 +43,6 @@ std::vector Diagnostics) = 0; }; -class FileSystemProvider { -public: - virtual ~FileSystemProvider() = default; - /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing. - /// Context::current() will be the context passed to the clang entrypoint, - /// such as addDocument(), and will also be propagated to result callbacks. - /// Embedders may use this to isolate filesystem accesses. - virtual IntrusiveRefCntPtr getFileSystem() = 0; -}; - -class RealFileSystemProvider : public FileSystemProvider { -public: - /// Returns getRealFileSystem(). - IntrusiveRefCntPtr getFileSystem() override; -}; - /// Provides API to manage ASTs for a collection of C++ files and request /// various language features. /// Currently supports async diagnostics, code completion, formatting and goto Index: clang-tools-extra/trunk/clangd/FSProvider.h === --- clang-tools-extra/trunk/clangd/FSProvider.h +++ clang-tools-extra/trunk/clangd/FSProvider.h @@ -0,0 +1,42 @@ +//===--- FSProvider.h - VFS provider for ClangdServer *- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H + +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" + +namespace clang { +namespace clangd { + +// Wrapper for vfs::FileSystem for use in multithreaded programs like clangd. +// As FileSystem is not threads
r336910 - [Tooling] Get working directory properly without assuming real file system.
Author: ioeric Date: Thu Jul 12 07:54:25 2018 New Revision: 336910 URL: http://llvm.org/viewvc/llvm-project?rev=336910&view=rev Log: [Tooling] Get working directory properly without assuming real file system. Modified: cfe/trunk/lib/Tooling/Tooling.cpp Modified: cfe/trunk/lib/Tooling/Tooling.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=336910&r1=336909&r2=336910&view=diff == --- cfe/trunk/lib/Tooling/Tooling.cpp (original) +++ cfe/trunk/lib/Tooling/Tooling.cpp Thu Jul 12 07:54:25 2018 @@ -411,10 +411,14 @@ int ClangTool::run(ToolAction *Action) { // This just needs to be some symbol in the binary. static int StaticSymbol; - llvm::SmallString<128> InitialDirectory; - if (std::error_code EC = llvm::sys::fs::current_path(InitialDirectory)) + std::string InitialDirectory; + if (llvm::ErrorOr CWD = + OverlayFileSystem->getCurrentWorkingDirectory()) { +InitialDirectory = std::move(*CWD); + } else { llvm::report_fatal_error("Cannot detect current path: " + - Twine(EC.message())); + Twine(CWD.getError().message())); + } // First insert all absolute paths into the in-memory VFS. These are global // for all compile commands. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49063: [libclang] Add support for ObjCObjectType
michaelwu added inline comments. Comment at: include/clang-c/Index.h:35 #define CINDEX_VERSION_MAJOR 0 #define CINDEX_VERSION_MINOR 49 yvvan wrote: > Please, increment the minor version (you are adding new functions) This is one in a series of patches that add features to libclang - would you be ok with incrementing at the end of the series? Comment at: include/clang-c/Index.h:3644 + */ +CINDEX_LINKAGE int clang_Type_getNumObjCProtocolRefs(CXType T); + yvvan wrote: > Other similar calls usually return unsigned int because they can't be below > zero. I was mimicking `clang_Type_getNumTemplateArguments` for this, which appears to be the most similar API. It uses 0 to n for the number of template arguments and -1 to indicate that it was called on the wrong type. If you're ok with the inconsistency, I'll make this unsigned. I also prefer having this unsigned. https://reviews.llvm.org/D49063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49244: Always search sysroot for GCC installs
greened created this revision. greened added reviewers: rsmith, mcrosier, danalbert. greened added a project: clang. Herald added a subscriber: cfe-commits. Previously, if clang was configured with -DGCC_INSTALL_PREFIX, then it would not search a provided sysroot for a gcc install. This caused a number of regression tests to fail. Add sysroot to the list of paths to search, even if a gcc toolchain is provided with cmake or a command-line option. The sysroot is searched after the provided install. Repository: rC Clang https://reviews.llvm.org/D49244 Files: lib/Driver/ToolChains/Gnu.cpp Index: lib/Driver/ToolChains/Gnu.cpp === --- lib/Driver/ToolChains/Gnu.cpp +++ lib/Driver/ToolChains/Gnu.cpp @@ -1670,6 +1670,15 @@ GCCToolchainDir = GCCToolchainDir.drop_back(); // remove the / Prefixes.push_back(GCCToolchainDir); + +// If we have a SysRoot, try that too as GCCToolchainDir may not +// have support for the target. This can happen, for example, if +// clang was configured with -DGCC_INSTALL_PREFIX and we run +// regression tests with --sysroot. +if (!D.SysRoot.empty()) { + Prefixes.push_back(D.SysRoot); + AddDefaultGCCPrefixes(TargetTriple, Prefixes, D.SysRoot); +} } else { // If we have a SysRoot, try that first. if (!D.SysRoot.empty()) { Index: lib/Driver/ToolChains/Gnu.cpp === --- lib/Driver/ToolChains/Gnu.cpp +++ lib/Driver/ToolChains/Gnu.cpp @@ -1670,6 +1670,15 @@ GCCToolchainDir = GCCToolchainDir.drop_back(); // remove the / Prefixes.push_back(GCCToolchainDir); + +// If we have a SysRoot, try that too as GCCToolchainDir may not +// have support for the target. This can happen, for example, if +// clang was configured with -DGCC_INSTALL_PREFIX and we run +// regression tests with --sysroot. +if (!D.SysRoot.empty()) { + Prefixes.push_back(D.SysRoot); + AddDefaultGCCPrefixes(TargetTriple, Prefixes, D.SysRoot); +} } else { // If we have a SysRoot, try that first. if (!D.SysRoot.empty()) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49245: [ASTImporter] Import implicit methods of existing class.
balazske created this revision. Herald added subscribers: cfe-commits, martong. Herald added a reviewer: a.sidorin. Herald added a reviewer: a.sidorin. When an already existing class is encountered during import, check if it has implicit methods that are missing in the existing one, and import these. The to-be-imported code may use the same class in different way than the existing (before the import) code. This may result in that there are implicit methods that are not generated for the existing code. Repository: rC Clang https://reviews.llvm.org/D49245 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2283,6 +2283,117 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()); } +class ImportImplicitMethods : public ASTImporterTestBase { +public: + static constexpr auto DefaultCode = R"( + struct A { int x; }; + void f() { +A a; +A a1(a); +A a2(A{}); +a = a1; +a = A{}; +a.~A(); + })"; + + template + void testImportOf( + const MatcherType &MethodMatcher, const char *Code = DefaultCode) { +test(MethodMatcher, Code, 1u); + } + + template + void testNoImportOf( + const MatcherType &MethodMatcher, const char *Code = DefaultCode) { +test(MethodMatcher, Code, 0u); + } + +private: + template + void test(const MatcherType &MethodMatcher, + const char *Code = DefaultCode, unsigned int ExpectedCount = 1u) { +auto ClassMatcher = cxxRecordDecl(unless(isImplicit())); + +Decl *ToTU = getToTuDecl(Code, Lang_CXX11); +auto *ToClass = FirstDeclMatcher().match( +ToTU, ClassMatcher); + +ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1); + +{ + CXXMethodDecl *Method = + FirstDeclMatcher().match(ToClass, MethodMatcher); + ToClass->removeDecl(Method); +} + +ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0); + +Decl *ImportedClass = nullptr; +{ + Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input1.cc"); + auto *FromClass = FirstDeclMatcher().match( + FromTU, ClassMatcher); + ImportedClass = Import(FromClass, Lang_CXX11); +} + +EXPECT_EQ(ToClass, ImportedClass); +EXPECT_EQ(DeclCounter().match(ToClass, MethodMatcher), +ExpectedCount); + } +}; + +TEST_P(ImportImplicitMethods, DefaultConstructor) { + testImportOf(cxxConstructorDecl(isDefaultConstructor())); +} + +TEST_P(ImportImplicitMethods, CopyConstructor) { + testImportOf(cxxConstructorDecl(isCopyConstructor())); +} + +TEST_P(ImportImplicitMethods, MoveConstructor) { + testImportOf(cxxConstructorDecl(isMoveConstructor())); +} + +TEST_P(ImportImplicitMethods, Destructor) { + testImportOf(cxxDestructorDecl()); +} + +TEST_P(ImportImplicitMethods, CopyAssignment) { + testImportOf(cxxMethodDecl(isCopyAssignmentOperator())); +} + +TEST_P(ImportImplicitMethods, MoveAssignment) { + testImportOf(cxxMethodDecl(isMoveAssignmentOperator())); +} + +TEST_P(ImportImplicitMethods, DoNotImportUserProvided) { + auto Code = R"( + struct A { A() { int x; } }; + )"; + testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code); +} + +TEST_P(ImportImplicitMethods, DoNotImportDefault) { + auto Code = R"( + struct A { A() = default; }; + )"; + testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code); +} + +TEST_P(ImportImplicitMethods, DoNotImportDeleted) { + auto Code = R"( + struct A { A() = delete; }; + )"; + testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code); +} + +TEST_P(ImportImplicitMethods, DoNotImportOtherMethod) { + auto Code = R"( + struct A { void f() { } }; + )"; + testNoImportOf(cxxMethodDecl(hasName("f")), Code); +} + TEST_P(ASTImporterTestBase, ImportOfEquivalentRecord) { Decl *ToR1; { Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -228,6 +228,7 @@ void ImportDeclarationNameLoc(const DeclarationNameInfo &From, DeclarationNameInfo& To); void ImportDeclContext(DeclContext *FromDC, bool ForceImport = false); +void ImportImplicitMethods(const CXXRecordDecl *From, CXXRecordDecl *To); bool ImportCastPath(CastExpr *E, CXXCastPath &Path); @@ -1235,6 +1236,16 @@ Importer.Import(From); } +void ASTNodeImporter::ImportImplicitMethods( +const CXXRecordDecl *From, CXXRecordDecl *To) { + assert(From->isCompleteDefinition() && To->getDefinition() == To && + "Import implicit methods to or from non-definition"); + + for (CXXMethodDecl *FromM : From->methods()) +if (FromM->isImplicit()) + Importer.Import(FromM); +} + static void setTypedefNameForAnonDecl(TagDecl *From, Ta
[PATCH] D49253: [clangd] Fix category in clangd-vscode's package.json
simark created this revision. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. When opening package.json, vscode shows: Use 'Programming Languages' instead Replacing "Languages" with this fixes it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49253 Files: clangd/clients/clangd-vscode/package.json Index: clangd/clients/clangd-vscode/package.json === --- clangd/clients/clangd-vscode/package.json +++ clangd/clients/clangd-vscode/package.json @@ -9,7 +9,7 @@ "vscode": "^1.18.0" }, "categories": [ -"Languages", +"Programming Languages", "Linters", "Snippets" ], Index: clangd/clients/clangd-vscode/package.json === --- clangd/clients/clangd-vscode/package.json +++ clangd/clients/clangd-vscode/package.json @@ -9,7 +9,7 @@ "vscode": "^1.18.0" }, "categories": [ -"Languages", +"Programming Languages", "Linters", "Snippets" ], ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49253: [clangd] Fix category in clangd-vscode's package.json
simark updated this revision to Diff 155213. simark added a comment. - [clangd] JSONTracer: flush after writing event Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49253 Files: clangd/Trace.cpp clangd/clients/clangd-vscode/package.json Index: clangd/clients/clangd-vscode/package.json === --- clangd/clients/clangd-vscode/package.json +++ clangd/clients/clangd-vscode/package.json @@ -9,7 +9,7 @@ "vscode": "^1.18.0" }, "categories": [ -"Languages", +"Programming Languages", "Linters", "Snippets" ], Index: clangd/Trace.cpp === --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -151,6 +151,7 @@ Event["ph"] = Phase; Out << Sep << formatv(JSONFormat, json::Value(std::move(Event))); Sep = ",\n"; +Out.flush(); } // If we haven't already, emit metadata describing this thread. Index: clangd/clients/clangd-vscode/package.json === --- clangd/clients/clangd-vscode/package.json +++ clangd/clients/clangd-vscode/package.json @@ -9,7 +9,7 @@ "vscode": "^1.18.0" }, "categories": [ -"Languages", +"Programming Languages", "Linters", "Snippets" ], Index: clangd/Trace.cpp === --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -151,6 +151,7 @@ Event["ph"] = Phase; Out << Sep << formatv(JSONFormat, json::Value(std::move(Event))); Sep = ",\n"; +Out.flush(); } // If we haven't already, emit metadata describing this thread. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336919 - [clang-tidy/ObjC] Add SQL to list of acronyms
Author: benhamilton Date: Thu Jul 12 10:32:55 2018 New Revision: 336919 URL: http://llvm.org/viewvc/llvm-project?rev=336919&view=rev Log: [clang-tidy/ObjC] Add SQL to list of acronyms Summary: SQL is a common acronym. Reviewers: Wizard, hokein Reviewed By: Wizard, hokein Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D49190 Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp?rev=336919&r1=336918&r2=336919&view=diff == --- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Thu Jul 12 10:32:55 2018 @@ -98,6 +98,7 @@ constexpr llvm::StringLiteral DefaultSpe "SC", "SDK", "SHA", +"SQL", "SSO", "TCP", "TIFF", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49190: [clang-tidy/ObjC] Add SQL to list of acronyms
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE336919: [clang-tidy/ObjC] Add SQL to list of acronyms (authored by benhamilton, committed by ). Changed prior to commit: https://reviews.llvm.org/D49190?vs=155004&id=155216#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49190 Files: clang-tidy/objc/PropertyDeclarationCheck.cpp Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -98,6 +98,7 @@ "SC", "SDK", "SHA", +"SQL", "SSO", "TCP", "TIFF", Index: clang-tidy/objc/PropertyDeclarationCheck.cpp === --- clang-tidy/objc/PropertyDeclarationCheck.cpp +++ clang-tidy/objc/PropertyDeclarationCheck.cpp @@ -98,6 +98,7 @@ "SC", "SDK", "SHA", +"SQL", "SSO", "TCP", "TIFF", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336920 - Revert "[modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts"
Author: bruno Date: Thu Jul 12 10:38:48 2018 New Revision: 336920 URL: http://llvm.org/viewvc/llvm-project?rev=336920&view=rev Log: Revert "[modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts" This reverts commit f40124d4f05ecf4f880cf4e8f26922d861f705f3 / r336660. This change shouldn't be affecting `@import` behavior, but turns out it is: https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/2800/consoleFull#-12570166563122a513-f36a-4c87-8ed7-cbc36a1ec144 Working on a reduced testcase for this, reverting in the meantime. rdar://problem/4210 Removed: cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap cfe/trunk/test/Modules/autoload-subdirectory.cpp Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Lex/HeaderSearch.cpp cfe/trunk/lib/Serialization/ASTReader.cpp Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=336920&r1=336919&r2=336920&view=diff == --- cfe/trunk/include/clang/Lex/HeaderSearch.h (original) +++ cfe/trunk/include/clang/Lex/HeaderSearch.h Thu Jul 12 10:38:48 2018 @@ -529,12 +529,8 @@ public: /// search directories to produce a module definition. If not, this lookup /// will only return an already-known module. /// - /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps - /// in subdirectories. - /// /// \returns The module with the given name. - Module *lookupModule(StringRef ModuleName, bool AllowSearch = true, - bool AllowExtraModuleMapSearch = false); + Module *lookupModule(StringRef ModuleName, bool AllowSearch = true); /// Try to find a module map file in the given directory, returning /// \c nullptr if none is found. @@ -599,12 +595,8 @@ private: /// but for compatibility with some buggy frameworks, additional attempts /// may be made to find the module under a related-but-different search-name. /// - /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps - /// in subdirectories. - /// /// \returns The module named ModuleName. - Module *lookupModule(StringRef ModuleName, StringRef SearchName, - bool AllowExtraModuleMapSearch = false); + Module *lookupModule(StringRef ModuleName, StringRef SearchName); /// Retrieve a module with the given name, which may be part of the /// given framework. Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=336920&r1=336919&r2=336920&view=diff == --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu Jul 12 10:38:48 2018 @@ -1653,10 +1653,8 @@ CompilerInstance::loadModule(SourceLocat // Retrieve the cached top-level module. Module = Known->second; } else if (ModuleName == getLangOpts().CurrentModule) { -// This is the module we're building. -Module = PP->getHeaderSearchInfo().lookupModule( -ModuleName, /*AllowSearch*/ true, -/*AllowExtraModuleMapSearch*/ !IsInclusionDirective); +// This is the module we're building. +Module = PP->getHeaderSearchInfo().lookupModule(ModuleName); /// FIXME: perhaps we should (a) look for a module using the module name // to file map (PrebuiltModuleFiles) and (b) diagnose if still not found? //if (Module == nullptr) { @@ -1668,8 +1666,7 @@ CompilerInstance::loadModule(SourceLocat Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first; } else { // Search for a module with the given name. -Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true, -!IsInclusionDirective); +Module = PP->getHeaderSearchInfo().lookupModule(ModuleName); HeaderSearchOptions &HSOpts = PP->getHeaderSearchInfo().getHeaderSearchOpts(); @@ -1746,8 +1743,7 @@ CompilerInstance::loadModule(SourceLocat ImportLoc, ARRFlags)) { case ASTReader::Success: { if (Source != ModuleCache && !Module) { -Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true, -!IsInclusionDirective); +Module = PP->getHeaderSearchInfo().lookupModule(ModuleName); if (!Module || !Module->getASTFile() || FileMgr->getFile(
[PATCH] D39679: [C++11] Fix warning when dropping cv-qualifiers when assigning to a reference with a braced initializer list
Rakete closed this revision. Rakete added a comment. Committed in r336922 :) https://reviews.llvm.org/D39679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49253: [clangd] Fix category in clangd-vscode's package.json
simark updated this revision to Diff 155232. simark added a comment. Oops. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49253 Files: clangd/clients/clangd-vscode/package.json Index: clangd/clients/clangd-vscode/package.json === --- clangd/clients/clangd-vscode/package.json +++ clangd/clients/clangd-vscode/package.json @@ -9,7 +9,7 @@ "vscode": "^1.18.0" }, "categories": [ -"Languages", +"Programming Languages", "Linters", "Snippets" ], Index: clangd/clients/clangd-vscode/package.json === --- clangd/clients/clangd-vscode/package.json +++ clangd/clients/clangd-vscode/package.json @@ -9,7 +9,7 @@ "vscode": "^1.18.0" }, "categories": [ -"Languages", +"Programming Languages", "Linters", "Snippets" ], ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49260: [clangd] JSONTracer: flush after writing event
simark created this revision. Herald added subscribers: cfe-commits, omtcyfz, jkorous, MaskRay, ioeric, ilya-biryukov. Let's say I use "CLANGD_TRACE=/tmp/clangd.json" and "tail -F /tmp/clangd.json", I'll often see unfinished lines, where the rest of the data is still in clangd's output buffer. Doing a flush in rawEvent makes sure we can see all the data immediatly. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49260 Files: clangd/Trace.cpp Index: clangd/Trace.cpp === --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -151,6 +151,7 @@ Event["ph"] = Phase; Out << Sep << formatv(JSONFormat, json::Value(std::move(Event))); Sep = ",\n"; +Out.flush(); } // If we haven't already, emit metadata describing this thread. Index: clangd/Trace.cpp === --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -151,6 +151,7 @@ Event["ph"] = Phase; Out << Sep << formatv(JSONFormat, json::Value(std::move(Event))); Sep = ",\n"; +Out.flush(); } // If we haven't already, emit metadata describing this thread. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336928 - [Tooling] Make standalone executor support user-provided vfs.
Author: ioeric Date: Thu Jul 12 11:32:11 2018 New Revision: 336928 URL: http://llvm.org/viewvc/llvm-project?rev=336928&view=rev Log: [Tooling] Make standalone executor support user-provided vfs. Modified: cfe/trunk/include/clang/Tooling/StandaloneExecution.h cfe/trunk/lib/Tooling/StandaloneExecution.cpp Modified: cfe/trunk/include/clang/Tooling/StandaloneExecution.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/StandaloneExecution.h?rev=336928&r1=336927&r2=336928&view=diff == --- cfe/trunk/include/clang/Tooling/StandaloneExecution.h (original) +++ cfe/trunk/include/clang/Tooling/StandaloneExecution.h Thu Jul 12 11:32:11 2018 @@ -37,6 +37,7 @@ public: StandaloneToolExecutor( const CompilationDatabase &Compilations, llvm::ArrayRef SourcePaths, + IntrusiveRefCntPtr BaseFS = vfs::getRealFileSystem(), std::shared_ptr PCHContainerOps = std::make_shared()); Modified: cfe/trunk/lib/Tooling/StandaloneExecution.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/StandaloneExecution.cpp?rev=336928&r1=336927&r2=336928&view=diff == --- cfe/trunk/lib/Tooling/StandaloneExecution.cpp (original) +++ cfe/trunk/lib/Tooling/StandaloneExecution.cpp Thu Jul 12 11:32:11 2018 @@ -30,9 +30,11 @@ static ArgumentsAdjuster getDefaultArgum StandaloneToolExecutor::StandaloneToolExecutor( const CompilationDatabase &Compilations, llvm::ArrayRef SourcePaths, +IntrusiveRefCntPtr BaseFS, std::shared_ptr PCHContainerOps) -: Tool(Compilations, SourcePaths), Context(&Results), - ArgsAdjuster(getDefaultArgumentsAdjusters()) { +: Tool(Compilations, SourcePaths, std::move(PCHContainerOps), + std::move(BaseFS)), + Context(&Results), ArgsAdjuster(getDefaultArgumentsAdjusters()) { // Use self-defined default argument adjusters instead of the default // adjusters that come with the old `ClangTool`. Tool.clearArgumentsAdjusters(); @@ -43,7 +45,7 @@ StandaloneToolExecutor::StandaloneToolEx std::shared_ptr PCHContainerOps) : OptionsParser(std::move(Options)), Tool(OptionsParser->getCompilations(), OptionsParser->getSourcePathList(), - PCHContainerOps), + std::move(PCHContainerOps)), Context(&Results), ArgsAdjuster(getDefaultArgumentsAdjusters()) { Tool.clearArgumentsAdjusters(); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37442: [C++17] Disallow lambdas in template parameters (PR33696).
Rakete updated this revision to Diff 155240. Rakete added a comment. Format lines longer than 80 characters correctly. Repository: rC Clang https://reviews.llvm.org/D37442 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseStmt.cpp lib/Parse/ParseTemplate.cpp lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/TreeTransform.h test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp Index: test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp === --- /dev/null +++ test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -std=c++17 %s -verify + +template struct Nothing {}; + +void pr33696() { +Nothing<[]() { return 0; }()> nothing; // expected-error{{a lambda expression cannot appear in this context}} +} Index: lib/Sema/TreeTransform.h === --- lib/Sema/TreeTransform.h +++ lib/Sema/TreeTransform.h @@ -3859,6 +3859,10 @@ bool TreeTransform::TransformTemplateArgument( const TemplateArgumentLoc &Input, TemplateArgumentLoc &Output, bool Uneval) { + EnterExpressionEvaluationContext EEEC( + SemaRef, Sema::ExpressionEvaluationContext::ConstantEvaluated, + /*LambdaContextDecl=*/nullptr, /*ExprContext=*/ + Sema::ExpressionEvaluationContextRecord::EK_TemplateArgument); const TemplateArgument &Arg = Input.getArgument(); switch (Arg.getKind()) { case TemplateArgument::Null: @@ -5494,7 +5498,7 @@ // decltype expressions are not potentially evaluated contexts EnterExpressionEvaluationContext Unevaluated( SemaRef, Sema::ExpressionEvaluationContext::Unevaluated, nullptr, - /*IsDecltype=*/true); + Sema::ExpressionEvaluationContextRecord::EK_Decltype); ExprResult E = getDerived().TransformExpr(T->getUnderlyingExpr()); if (E.isInvalid()) Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -6434,7 +6434,8 @@ if (RD->isInvalidDecl() || RD->isDependentContext()) return E; - bool IsDecltype = ExprEvalContexts.back().IsDecltype; + bool IsDecltype = ExprEvalContexts.back().ExprContext == +ExpressionEvaluationContextRecord::EK_Decltype; CXXDestructorDecl *Destructor = IsDecltype ? nullptr : LookupDestructor(RD); if (Destructor) { @@ -6516,7 +6517,9 @@ /// are omitted for the 'topmost' call in the decltype expression. If the /// topmost call bound a temporary, strip that temporary off the expression. ExprResult Sema::ActOnDecltypeExpression(Expr *E) { - assert(ExprEvalContexts.back().IsDecltype && "not in a decltype expression"); + assert(ExprEvalContexts.back().ExprContext == + ExpressionEvaluationContextRecord::EK_Decltype && + "not in a decltype expression"); // C++11 [expr.call]p11: // If a function call is a prvalue of object type, @@ -6558,7 +6561,8 @@ TopBind = nullptr; // Disable the special decltype handling now. - ExprEvalContexts.back().IsDecltype = false; + ExprEvalContexts.back().ExprContext = + ExpressionEvaluationContextRecord::EK_Other; // In MS mode, don't perform any extra checking of call return types within a // decltype expression. Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -14132,53 +14132,53 @@ } void -Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - Decl *LambdaContextDecl, - bool IsDecltype) { +Sema::PushExpressionEvaluationContext( +ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl, +ExpressionEvaluationContextRecord::ExpressionKind ExprContext) { ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup, -LambdaContextDecl, IsDecltype); +LambdaContextDecl, ExprContext); Cleanup.reset(); if (!MaybeODRUseExprs.empty()) std::swap(MaybeODRUseExprs, ExprEvalContexts.back().SavedMaybeODRUseExprs); } void -Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - ReuseLambdaContextDecl_t, - bool IsDecltype) { +Sema::PushExpressionEvaluationContext( +ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t, +ExpressionEvaluationContextRecord::ExpressionKind ExprContext) { Decl *ClosureContextDecl = ExprEvalContexts.back().ManglingContextDecl; - PushExpressionEvaluationContext(NewC
r336930 - [C++17] Disallow lambdas in template parameters (PR33696).
Author: rakete Date: Thu Jul 12 11:45:41 2018 New Revision: 336930 URL: http://llvm.org/viewvc/llvm-project?rev=336930&view=rev Log: [C++17] Disallow lambdas in template parameters (PR33696). Summary: This revision disallows lambdas in template parameters, as reported in PR33696. Reviewers: rsmith Reviewed By: rsmith Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D37442 Added: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Parse/ParseDecl.cpp cfe/trunk/lib/Parse/ParseDeclCXX.cpp cfe/trunk/lib/Parse/ParseStmt.cpp cfe/trunk/lib/Parse/ParseTemplate.cpp cfe/trunk/lib/Sema/Sema.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/lib/Sema/TreeTransform.h Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=336930&r1=336929&r2=336930&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jul 12 11:45:41 2018 @@ -6493,6 +6493,8 @@ let CategoryName = "Lambda Issue" in { "lambda expression in an unevaluated operand">; def err_lambda_in_constant_expression : Error< "a lambda expression may not appear inside of a constant expression">; + def err_lambda_in_invalid_context : Error< +"a lambda expression cannot appear in this context">; def err_lambda_return_init_list : Error< "cannot deduce lambda return type from initializer list">; def err_lambda_capture_default_arg : Error< Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=336930&r1=336929&r2=336930&view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Thu Jul 12 11:45:41 2018 @@ -979,15 +979,21 @@ public: /// expressions for which we have deferred checking the destructor. SmallVector DelayedDecltypeBinds; +/// \brief Describes whether we are in an expression constext which we have +/// to handle differently. +enum ExpressionKind { + EK_Decltype, EK_TemplateArgument, EK_Other +} ExprContext; + ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context, unsigned NumCleanupObjects, CleanupInfo ParentCleanup, Decl *ManglingContextDecl, - bool IsDecltype) - : Context(Context), ParentCleanup(ParentCleanup), -IsDecltype(IsDecltype), NumCleanupObjects(NumCleanupObjects), -NumTypos(0), -ManglingContextDecl(ManglingContextDecl), MangleNumbering() { } + ExpressionKind ExprContext) +: Context(Context), ParentCleanup(ParentCleanup), + NumCleanupObjects(NumCleanupObjects), NumTypos(0), + ManglingContextDecl(ManglingContextDecl), MangleNumbering(), + ExprContext(ExprContext) {} /// Retrieve the mangling numbering context, used to consistently /// number constructs like lambdas for mangling. @@ -3989,13 +3995,15 @@ public: void DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc, ArrayRef Args); - void PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - Decl *LambdaContextDecl = nullptr, - bool IsDecltype = false); + void PushExpressionEvaluationContext( + ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl = nullptr, + ExpressionEvaluationContextRecord::ExpressionKind Type = + ExpressionEvaluationContextRecord::EK_Other); enum ReuseLambdaContextDecl_t { ReuseLambdaContextDecl }; - void PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - ReuseLambdaContextDecl_t, - bool IsDecltype = false); + void PushExpressionEvaluationContext( + ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t, + ExpressionEvaluationContextRecord::ExpressionKind Type = + ExpressionEvaluationContextRecord::EK_Other); void PopExpressionEvaluationContext(); void DiscardCleanupsInEvaluationContext(); @@ -10748,25 +10756,25 @@ class EnterExpressionEvaluationContext { bool Entered = true; public: - - EnterExpressionEvaluationContext(Sema &Actions, - Sema::ExpressionEvaluationContext NewContext, -
[PATCH] D37442: [C++17] Disallow lambdas in template parameters (PR33696).
This revision was automatically updated to reflect the committed changes. Closed by commit rC336930: [C++17] Disallow lambdas in template parameters (PR33696). (authored by Rakete, committed by ). Changed prior to commit: https://reviews.llvm.org/D37442?vs=155240&id=155243#toc Repository: rC Clang https://reviews.llvm.org/D37442 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseStmt.cpp lib/Parse/ParseTemplate.cpp lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/TreeTransform.h test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -979,15 +979,21 @@ /// expressions for which we have deferred checking the destructor. SmallVector DelayedDecltypeBinds; +/// \brief Describes whether we are in an expression constext which we have +/// to handle differently. +enum ExpressionKind { + EK_Decltype, EK_TemplateArgument, EK_Other +} ExprContext; + ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context, unsigned NumCleanupObjects, CleanupInfo ParentCleanup, Decl *ManglingContextDecl, - bool IsDecltype) - : Context(Context), ParentCleanup(ParentCleanup), -IsDecltype(IsDecltype), NumCleanupObjects(NumCleanupObjects), -NumTypos(0), -ManglingContextDecl(ManglingContextDecl), MangleNumbering() { } + ExpressionKind ExprContext) +: Context(Context), ParentCleanup(ParentCleanup), + NumCleanupObjects(NumCleanupObjects), NumTypos(0), + ManglingContextDecl(ManglingContextDecl), MangleNumbering(), + ExprContext(ExprContext) {} /// Retrieve the mangling numbering context, used to consistently /// number constructs like lambdas for mangling. @@ -3989,13 +3995,15 @@ void DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc, ArrayRef Args); - void PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - Decl *LambdaContextDecl = nullptr, - bool IsDecltype = false); + void PushExpressionEvaluationContext( + ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl = nullptr, + ExpressionEvaluationContextRecord::ExpressionKind Type = + ExpressionEvaluationContextRecord::EK_Other); enum ReuseLambdaContextDecl_t { ReuseLambdaContextDecl }; - void PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - ReuseLambdaContextDecl_t, - bool IsDecltype = false); + void PushExpressionEvaluationContext( + ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t, + ExpressionEvaluationContextRecord::ExpressionKind Type = + ExpressionEvaluationContextRecord::EK_Other); void PopExpressionEvaluationContext(); void DiscardCleanupsInEvaluationContext(); @@ -10748,25 +10756,25 @@ bool Entered = true; public: - - EnterExpressionEvaluationContext(Sema &Actions, - Sema::ExpressionEvaluationContext NewContext, - Decl *LambdaContextDecl = nullptr, - bool IsDecltype = false, - bool ShouldEnter = true) + EnterExpressionEvaluationContext( + Sema &Actions, Sema::ExpressionEvaluationContext NewContext, + Decl *LambdaContextDecl = nullptr, + Sema::ExpressionEvaluationContextRecord::ExpressionKind ExprContext = + Sema::ExpressionEvaluationContextRecord::EK_Other, + bool ShouldEnter = true) : Actions(Actions), Entered(ShouldEnter) { if (Entered) Actions.PushExpressionEvaluationContext(NewContext, LambdaContextDecl, - IsDecltype); + ExprContext); } - EnterExpressionEvaluationContext(Sema &Actions, - Sema::ExpressionEvaluationContext NewContext, - Sema::ReuseLambdaContextDecl_t, - bool IsDecltype = false) -: Actions(Actions) { -Actions.PushExpressionEvaluationContext(NewContext, -Sema::ReuseLambdaContextDecl, -IsDecltype); + EnterExpressionEvaluationContext( + Sema &Actions, Sema::ExpressionEvaluationContext NewContext, + Sema::ReuseLambdaContextDecl_t, +
[PATCH] D37442: [C++17] Disallow lambdas in template parameters (PR33696).
This revision was automatically updated to reflect the committed changes. Closed by commit rL336930: [C++17] Disallow lambdas in template parameters (PR33696). (authored by Rakete, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D37442 Files: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Parse/ParseDecl.cpp cfe/trunk/lib/Parse/ParseDeclCXX.cpp cfe/trunk/lib/Parse/ParseStmt.cpp cfe/trunk/lib/Parse/ParseTemplate.cpp cfe/trunk/lib/Sema/Sema.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/lib/Sema/TreeTransform.h cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp Index: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp === --- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp +++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -std=c++17 %s -verify + +template struct Nothing {}; + +void pr33696() { +Nothing<[]() { return 0; }()> nothing; // expected-error{{a lambda expression cannot appear in this context}} +} Index: cfe/trunk/lib/Sema/TreeTransform.h === --- cfe/trunk/lib/Sema/TreeTransform.h +++ cfe/trunk/lib/Sema/TreeTransform.h @@ -3859,6 +3859,10 @@ bool TreeTransform::TransformTemplateArgument( const TemplateArgumentLoc &Input, TemplateArgumentLoc &Output, bool Uneval) { + EnterExpressionEvaluationContext EEEC( + SemaRef, Sema::ExpressionEvaluationContext::ConstantEvaluated, + /*LambdaContextDecl=*/nullptr, /*ExprContext=*/ + Sema::ExpressionEvaluationContextRecord::EK_TemplateArgument); const TemplateArgument &Arg = Input.getArgument(); switch (Arg.getKind()) { case TemplateArgument::Null: @@ -5494,7 +5498,7 @@ // decltype expressions are not potentially evaluated contexts EnterExpressionEvaluationContext Unevaluated( SemaRef, Sema::ExpressionEvaluationContext::Unevaluated, nullptr, - /*IsDecltype=*/true); + Sema::ExpressionEvaluationContextRecord::EK_Decltype); ExprResult E = getDerived().TransformExpr(T->getUnderlyingExpr()); if (E.isInvalid()) Index: cfe/trunk/lib/Sema/Sema.cpp === --- cfe/trunk/lib/Sema/Sema.cpp +++ cfe/trunk/lib/Sema/Sema.cpp @@ -163,7 +163,7 @@ ExprEvalContexts.emplace_back( ExpressionEvaluationContext::PotentiallyEvaluated, 0, CleanupInfo{}, - nullptr, false); + nullptr, ExpressionEvaluationContextRecord::EK_Other); PreallocatedFunctionScope.reset(new FunctionScopeInfo(Diags)); Index: cfe/trunk/lib/Sema/SemaExpr.cpp === --- cfe/trunk/lib/Sema/SemaExpr.cpp +++ cfe/trunk/lib/Sema/SemaExpr.cpp @@ -14132,53 +14132,53 @@ } void -Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - Decl *LambdaContextDecl, - bool IsDecltype) { +Sema::PushExpressionEvaluationContext( +ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl, +ExpressionEvaluationContextRecord::ExpressionKind ExprContext) { ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup, -LambdaContextDecl, IsDecltype); +LambdaContextDecl, ExprContext); Cleanup.reset(); if (!MaybeODRUseExprs.empty()) std::swap(MaybeODRUseExprs, ExprEvalContexts.back().SavedMaybeODRUseExprs); } void -Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext, - ReuseLambdaContextDecl_t, - bool IsDecltype) { +Sema::PushExpressionEvaluationContext( +ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t, +ExpressionEvaluationContextRecord::ExpressionKind ExprContext) { Decl *ClosureContextDecl = ExprEvalContexts.back().ManglingContextDecl; - PushExpressionEvaluationContext(NewContext, ClosureContextDecl, IsDecltype); + PushExpressionEvaluationContext(NewContext, ClosureContextDecl, ExprContext); } void Sema::PopExpressionEvaluationContext() { ExpressionEvaluationContextRecord& Rec = ExprEvalContexts.back(); unsigned NumTypos = Rec.NumTypos; if (!Rec.Lambdas.empty()) { -if (Rec.isUnevaluated() || Rec.isConstantEvaluated()) { +using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind; +if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || Rec.isUnevaluated() || +(Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {
r336931 - Add tests for function conversions in conversion function template
Author: rsmith Date: Thu Jul 12 11:49:13 2018 New Revision: 336931 URL: http://llvm.org/viewvc/llvm-project?rev=336931&view=rev Log: Add tests for function conversions in conversion function template deduction. Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.conv/p4.cpp Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=336931&r1=336930&r2=336931&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Thu Jul 12 11:49:13 2018 @@ -1645,6 +1645,9 @@ DeduceTemplateArgumentsByTypeMatch(Sema } } // FIXME: Detect non-deduced exception specification mismatches? + // + // Careful about [temp.deduct.call] and [temp.deduct.conv], which allow + // top-level differences in noexcept-specifications. return Sema::TDK_Success; } Modified: cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.conv/p4.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.conv/p4.cpp?rev=336931&r1=336930&r2=336931&view=diff == --- cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.conv/p4.cpp (original) +++ cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.conv/p4.cpp Thu Jul 12 11:49:13 2018 @@ -129,4 +129,21 @@ namespace non_ptr_ref_cv_qual { }; int (&test_conv_to_arr_1)[3] = ConvToArr(); // ok const int (&test_conv_to_arr_2)[3] = ConvToArr(); // ok, with qualification conversion + +#if __cplusplus >= 201702L + template using Function = T(U...) noexcept(Noexcept); + template struct ConvToFunction { +template operator Function&(); // expected-note {{candidate}} + }; + void (&fn1)(int) noexcept(false) = ConvToFunction(); + void (&fn2)(int) noexcept(true) = ConvToFunction(); // expected-error {{no viable}} + void (&fn3)(int) noexcept(false) = ConvToFunction(); + void (&fn4)(int) noexcept(true) = ConvToFunction(); + + struct ConvToFunctionDeducingNoexcept { +template operator Function&(); + }; + void (&fn5)(int) noexcept(false) = ConvToFunctionDeducingNoexcept(); + void (&fn6)(int) noexcept(true) = ConvToFunctionDeducingNoexcept(); +#endif } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48845: [Sema] Add fixit for unused lambda captures
acomminos updated this revision to Diff 155246. acomminos added a comment. Thanks for the feedback! This diff switches to using a source range for captures provided by the parser, which is more accurate, future-proof, and correctly handles macros. Repository: rC Clang https://reviews.llvm.org/D48845 Files: include/clang/Sema/DeclSpec.h include/clang/Sema/ScopeInfo.h include/clang/Sema/Sema.h lib/Parse/ParseExprCXX.cpp lib/Sema/SemaLambda.cpp test/FixIt/fixit-unused-lambda-capture.cpp Index: test/FixIt/fixit-unused-lambda-capture.cpp === --- /dev/null +++ test/FixIt/fixit-unused-lambda-capture.cpp @@ -0,0 +1,71 @@ +// RUN: cp %s %t +// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -std=c++1z -fixit %t +// RUN: grep -v CHECK %t | FileCheck %s + +void test() { + int i = 0; + int j = 0; + int k = 0; + int c = 10; + int a[c]; + + [i,j] { return i; }; + // CHECK: [i] { return i; }; + [i,j] { return j; }; + // CHECK: [j] { return j; }; + [i,j,k] {}; + // CHECK: [] {}; + [i,j,k] { return i + j; }; + // CHECK: [i,j] { return i + j; }; + [i,j,k] { return j + k; }; + // CHECK: [j,k] { return j + k; }; + [i,j,k] { return i + k; }; + // CHECK: [i,k] { return i + k; }; + [i,j,k] { return i + j + k; }; + // CHECK: [i,j,k] { return i + j + k; }; + [&,i] { return k; }; + // CHECK: [&] { return k; }; + [=,&i] { return k; }; + // CHECK: [=] { return k; }; + [=,&i,&j] { return j; }; + // CHECK: [=,&j] { return j; }; + [=,&i,&j] { return i; }; + // CHECK: [=,&i] { return i; }; + [z = i] {}; + // CHECK: [] {}; + [i,z = i,j] { return z; }; + // CHECK: [z = i] { return z; }; + [&a] {}; + // CHECK: [] {}; + + #define I_MACRO() i + #define I_REF_MACRO() &i + [I_MACRO()] {}; + // CHECK: [] {}; + [I_MACRO(),j] { return j; }; + // CHECK: [j] { return j; }; + [j,I_MACRO()] { return j; }; + // CHECK: [j] { return j; }; + [I_REF_MACRO(),j] { return j; }; + // CHECK: [j] { return j; }; + [j,I_REF_MACRO()] { return j; }; + // CHECK: [j] { return j; }; +} + +class ThisTest { + void test() { +int i = 0; +[this] {}; +// CHECK: [] {}; +[i,this] { return i; }; +// CHECK: [i] { return i; }; +[this,i] { return i; }; +// CHECK: [i] { return i; }; +[*this] {}; +// CHECK: [] {}; +[*this,i] { return i; }; +// CHECK: [i] { return i; }; +[i,*this] { return i; }; +// CHECK: [i] { return i; }; + } +}; Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -824,6 +824,7 @@ LSI->addCapture(Var, /*isBlock*/false, Var->getType()->isReferenceType(), /*isNested*/false, Var->getLocation(), SourceLocation(), Var->getType(), Var->getInit()); + return Field; } @@ -954,6 +955,9 @@ = Intro.Default == LCD_None? Intro.Range.getBegin() : Intro.DefaultLoc; for (auto C = Intro.Captures.begin(), E = Intro.Captures.end(); C != E; PrevCaptureLoc = C->Loc, ++C) { +LSI->ExplicitCaptureRanges[C - Intro.Captures.begin()] = +SourceRange(C->LocStart, C->LocEnd); + if (C->Kind == LCK_This || C->Kind == LCK_StarThis) { if (C->Kind == LCK_StarThis) Diag(C->Loc, !getLangOpts().CPlusPlus17 @@ -1478,7 +1482,8 @@ return false; } -void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) { +void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange, + const Capture &From) { if (CaptureHasSideEffects(From)) return; @@ -1491,6 +1496,7 @@ else diag << From.getVariable(); diag << From.isNonODRUsed(); + diag << FixItHint::CreateRemoval(CaptureRange); } ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, @@ -1532,19 +1538,47 @@ // Translate captures. auto CurField = Class->field_begin(); +// True if the current capture has a used capture or default before it. +bool CurHasPreviousCapture = CaptureDefault != LCD_None; +SourceLocation PrevCaptureLoc = CurHasPreviousCapture ? +CaptureDefaultLoc : IntroducerRange.getBegin(); + for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) { const Capture &From = LSI->Captures[I]; assert(!From.isBlockCapture() && "Cannot capture __block variables"); bool IsImplicit = I >= LSI->NumExplicitCaptures; + // Use source ranges of explicit captures for fixits where available. + SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I]; + if (CaptureRange.isInvalid()) { +CaptureRange = SourceRange(From.getLocation()); + } + // Warn about unused explicit captures. + bool IsCaptureUsed = true; if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) { // Initialized captures that are non-ODR used may not be eliminated.
r336933 - [Hexagon] Diagnose intrinsics not supported by selected CPU/HVX
Author: kparzysz Date: Thu Jul 12 11:54:04 2018 New Revision: 336933 URL: http://llvm.org/viewvc/llvm-project?rev=336933&view=rev Log: [Hexagon] Diagnose intrinsics not supported by selected CPU/HVX Added: cfe/trunk/test/Sema/builtins-hexagon-v55.c cfe/trunk/test/Sema/builtins-hexagon-v60.c cfe/trunk/test/Sema/builtins-hexagon-v62.c cfe/trunk/test/Sema/builtins-hexagon-v65.c cfe/trunk/test/Sema/builtins-hvx-none.c cfe/trunk/test/Sema/builtins-hvx-v60.c cfe/trunk/test/Sema/builtins-hvx-v62.c cfe/trunk/test/Sema/builtins-hvx-v65.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Basic/Targets/Hexagon.cpp cfe/trunk/lib/Sema/SemaChecking.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=336933&r1=336932&r2=336933&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jul 12 11:54:04 2018 @@ -8181,9 +8181,15 @@ def err_x86_builtin_invalid_rounding : E "invalid rounding argument">; def err_x86_builtin_invalid_scale : Error< "scale argument must be 1, 2, 4, or 8">; +def err_hexagon_builtin_unsupported_cpu : Error< + "builtin is not supported on this CPU">; +def err_hexagon_builtin_requires_hvx : Error< + "builtin requires HVX">; +def err_hexagon_builtin_unsupported_hvx : Error< + "builtin is not supported on this version of HVX">; + def err_builtin_target_unsupported : Error< "builtin is not supported on this target">; - def err_builtin_longjmp_unsupported : Error< "__builtin_longjmp is not supported for the current target">; def err_builtin_setjmp_unsupported : Error< Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=336933&r1=336932&r2=336933&view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Thu Jul 12 11:54:04 2018 @@ -10418,6 +10418,8 @@ private: bool CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); bool CheckHexagonBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); + bool CheckHexagonBuiltinCpu(unsigned BuiltinID, CallExpr *TheCall); + bool CheckHexagonBuiltinArgument(unsigned BuiltinID, CallExpr *TheCall); bool CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); bool CheckSystemZBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); bool CheckX86BuiltinRoundingOrSAE(unsigned BuiltinID, CallExpr *TheCall); Modified: cfe/trunk/lib/Basic/Targets/Hexagon.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Hexagon.cpp?rev=336933&r1=336932&r2=336933&view=diff == --- cfe/trunk/lib/Basic/Targets/Hexagon.cpp (original) +++ cfe/trunk/lib/Basic/Targets/Hexagon.cpp Thu Jul 12 11:54:04 2018 @@ -131,6 +131,10 @@ const Builtin::Info HexagonTargetInfo::B }; bool HexagonTargetInfo::hasFeature(StringRef Feature) const { + std::string VS = "hvxv" + HVXVersion; + if (Feature == VS) +return true; + return llvm::StringSwitch(Feature) .Case("hexagon", true) .Case("hvx", HasHVX) Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=336933&r1=336932&r2=336933&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Jul 12 11:54:04 2018 @@ -1731,8 +1731,791 @@ bool Sema::CheckAArch64BuiltinFunctionCa return SemaBuiltinConstantArgRange(TheCall, i, l, u + l); } -bool Sema::CheckHexagonBuiltinFunctionCall(unsigned BuiltinID, - CallExpr *TheCall) { +bool Sema::CheckHexagonBuiltinCpu(unsigned BuiltinID, CallExpr *TheCall) { + static const std::map> ValidCPU = { +{ Hexagon::BI__builtin_HEXAGON_A6_vcmpbeq_notany, {"v65"} }, +{ Hexagon::BI__builtin_HEXAGON_A6_vminub_RdP, {"v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_M6_vabsdiffb, {"v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_M6_vabsdiffub, {"v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_S6_rol_i_p_acc, {"v60", "v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_S6_rol_i_p_and, {"v60", "v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_S6_rol_i_p_nac, {"v60", "v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_S6_rol_i_p_or, {"v60", "v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_S6_rol_i_p, {"v60", "v62", "v65"} }, +{ Hexagon::BI__builtin_HEXAGON_S6_rol_i_p_xacc, {"v60", "v62", "v65"} }, +{ H
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
NoQ accepted this revision. NoQ added a comment. Looks good! Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:691 +StringRef getVariableName(const FieldDecl *Field) { + // If \p Field is a captured lambda variable, Field->getName() will return Szelethus wrote: > george.karpenkov wrote: > > Is this a member method? Then it should be prefixed with a class name. > > Currently it looks like you have a member method without an implementation > > and a separate C-style function? > It is a statically defined method, and you can see its forward declaration in > the same diff. > > Back when I started writing this checker, the amount of function laying in > the anonymous namespace was very few, but this has changed. I'll fix it > sometime because it's getting somewhat annoying (and is against the [[ > https://llvm.org/docs/CodingStandards.html | coding standards ]]). Yeah, just make them `static`, it's easier to read (https://llvm.org/docs/CodingStandards.html#anonymous-namespaces). https://reviews.llvm.org/D48291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48325: [analyzer][UninitializedObjectChecker] Support for MemberPointerTypes
NoQ accepted this revision. NoQ added a comment. Yay less code. https://reviews.llvm.org/D48325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336922 - [C++11] Fix warning when dropping cv-qualifiers when assigning to a reference with a braced initializer list
Author: rakete Date: Thu Jul 12 10:43:49 2018 New Revision: 336922 URL: http://llvm.org/viewvc/llvm-project?rev=336922&view=rev Log: [C++11] Fix warning when dropping cv-qualifiers when assigning to a reference with a braced initializer list Modified: cfe/trunk/lib/Sema/SemaInit.cpp cfe/trunk/test/SemaCXX/references.cpp Modified: cfe/trunk/lib/Sema/SemaInit.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=336922&r1=336921&r2=336922&view=diff == --- cfe/trunk/lib/Sema/SemaInit.cpp (original) +++ cfe/trunk/lib/Sema/SemaInit.cpp Thu Jul 12 10:43:49 2018 @@ -7567,6 +7567,19 @@ bool InitializationSequence::Diagnose(Se if (!Failed()) return false; + // When we want to diagnose only one element of a braced-init-list, + // we need to factor it out. + Expr *OnlyArg; + if (Args.size() == 1) { +auto *List = dyn_cast(Args[0]); +if (List && List->getNumInits() == 1) + OnlyArg = List->getInit(0); +else + OnlyArg = Args[0]; + } + else +OnlyArg = nullptr; + QualType DestType = Entity.getType(); switch (Failure) { case FK_TooManyInitsForReference: @@ -7627,7 +7640,7 @@ bool InitializationSequence::Diagnose(Se ? diag::err_array_init_different_type : diag::err_array_init_non_constant_array)) << DestType.getNonReferenceType() - << Args[0]->getType() + << OnlyArg->getType() << Args[0]->getSourceRange(); break; @@ -7638,7 +7651,7 @@ bool InitializationSequence::Diagnose(Se case FK_AddressOfOverloadFailed: { DeclAccessPair Found; -S.ResolveAddressOfOverloadedFunction(Args[0], +S.ResolveAddressOfOverloadedFunction(OnlyArg, DestType.getNonReferenceType(), true, Found); @@ -7646,9 +7659,9 @@ bool InitializationSequence::Diagnose(Se } case FK_AddressOfUnaddressableFunction: { -auto *FD = cast(cast(Args[0])->getDecl()); +auto *FD = cast(cast(OnlyArg)->getDecl()); S.checkAddressOfFunctionIsAvailable(FD, /*Complain=*/true, -Args[0]->getLocStart()); +OnlyArg->getLocStart()); break; } @@ -7658,11 +7671,11 @@ bool InitializationSequence::Diagnose(Se case OR_Ambiguous: if (Failure == FK_UserConversionOverloadFailed) S.Diag(Kind.getLocation(), diag::err_typecheck_ambiguous_condition) - << Args[0]->getType() << DestType + << OnlyArg->getType() << DestType << Args[0]->getSourceRange(); else S.Diag(Kind.getLocation(), diag::err_ref_init_ambiguous) - << DestType << Args[0]->getType() + << DestType << OnlyArg->getType() << Args[0]->getSourceRange(); FailedCandidateSet.NoteCandidates(S, OCD_ViableCandidates, Args); @@ -7672,10 +7685,10 @@ bool InitializationSequence::Diagnose(Se if (!S.RequireCompleteType(Kind.getLocation(), DestType.getNonReferenceType(), diag::err_typecheck_nonviable_condition_incomplete, - Args[0]->getType(), Args[0]->getSourceRange())) + OnlyArg->getType(), Args[0]->getSourceRange())) S.Diag(Kind.getLocation(), diag::err_typecheck_nonviable_condition) << (Entity.getKind() == InitializedEntity::EK_Result) - << Args[0]->getType() << Args[0]->getSourceRange() + << OnlyArg->getType() << Args[0]->getSourceRange() << DestType.getNonReferenceType(); FailedCandidateSet.NoteCandidates(S, OCD_AllCandidates, Args); @@ -7683,7 +7696,7 @@ bool InitializationSequence::Diagnose(Se case OR_Deleted: { S.Diag(Kind.getLocation(), diag::err_typecheck_deleted_function) -<< Args[0]->getType() << DestType.getNonReferenceType() +<< OnlyArg->getType() << DestType.getNonReferenceType() << Args[0]->getSourceRange(); OverloadCandidateSet::iterator Best; OverloadingResult Ovl @@ -7719,7 +7732,7 @@ bool InitializationSequence::Diagnose(Se : diag::err_lvalue_reference_bind_to_unrelated) << DestType.getNonReferenceType().isVolatileQualified() << DestType.getNonReferenceType() - << Args[0]->getType() + << OnlyArg->getType() << Args[0]->getSourceRange(); break; @@ -7744,12 +7757,12 @@ bool InitializationSequence::Diagnose(Se case FK_RValueReferenceBindingToLValue: S.Diag(Kind.getLocation(), diag::err_lvalue_to_rvalue_ref) - << DestType.getNonReferenceType() << Args[0]->getType() + << DestType.getNonReferenceType() << OnlyArg->getType() << Args[0]->getSourceRange(); break; case FK_ReferenceInitDropsQualifiers: { -QualType SourceType = Args[0]->getTyp
[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part
lebedev.ri added a comment. In https://reviews.llvm.org/D48958#1160479, @vsk wrote: > In https://reviews.llvm.org/D48958#1160435, @lebedev.ri wrote: > > > Thank you for taking a look! > > > > In https://reviews.llvm.org/D48958#1160381, @vsk wrote: > > > > > I have some minor comments but overall I think this is in good shape. It > > > would be great to see some compile-time numbers just to make sure this is > > > tractable. I'm pretty sure -fsanitize=null would fire more often across a > > > codebase than this check, so I don't anticipate a big surprise here. > > > > > > Could you please clarify, which numbers are you looking for, specifically? > > The time it takes to build llvm stage2 with `-fsanitize=implicit-cast`? > > Or the time it takes to build llvm stage3 with compiler built with > > `-fsanitize=implicit-cast`? > > > I had in mind measuring the difference between -fsanitize=undefined and > -fsanitize=undefined,implicit-cast, with a stage2 compiler. I think that > captures the expected use case: existing ubsan users enabling this new check. FWIW, i'm trying to look into optimizing these new IR patterns right now https://reviews.llvm.org/D49179 https://reviews.llvm.org/D49247. >> (The numbers won't be too representable, whole stage-1 takes ~40 minutes >> here...) > > Ah I see, I'll run a few builds and take a stab at it, then. Yes, please, thank you! Repository: rC Clang https://reviews.llvm.org/D48958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part
lebedev.ri added a comment. Thank you for taking a look! In https://reviews.llvm.org/D48958#1160381, @vsk wrote: > I have some minor comments but overall I think this is in good shape. It > would be great to see some compile-time numbers just to make sure this is > tractable. I'm pretty sure -fsanitize=null would fire more often across a > codebase than this check, so I don't anticipate a big surprise here. Could you please clarify, which numbers are you looking for, specifically? The time it takes to build llvm stage2 with `-fsanitize=implicit-cast`? Or the time it takes to build llvm stage3 with compiler built with `-fsanitize=implicit-cast`? (The numbers won't be too representable, whole stage-1 takes ~40 minutes here...) Comment at: lib/CodeGen/CGExprScalar.cpp:351 +ScalarConversionOpts() +: TreatBooleanAsSigned(false), + EmitImplicitIntegerTruncationChecks(false) {} vsk wrote: > Why not use default member initializers here (e.g, "bool a = false")? I'll double-check, but i'm pretty sure then there were some warnings when i did that, Or, the default needs to be defined in the actual declaration of `EmitScalarConversion()`, i think. Repository: rC Clang https://reviews.llvm.org/D48958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part
vsk added a comment. In https://reviews.llvm.org/D48958#1160435, @lebedev.ri wrote: > Thank you for taking a look! > > In https://reviews.llvm.org/D48958#1160381, @vsk wrote: > > > I have some minor comments but overall I think this is in good shape. It > > would be great to see some compile-time numbers just to make sure this is > > tractable. I'm pretty sure -fsanitize=null would fire more often across a > > codebase than this check, so I don't anticipate a big surprise here. > > > Could you please clarify, which numbers are you looking for, specifically? > The time it takes to build llvm stage2 with `-fsanitize=implicit-cast`? > Or the time it takes to build llvm stage3 with compiler built with > `-fsanitize=implicit-cast`? I had in mind measuring the difference between -fsanitize=undefined and -fsanitize=undefined,implicit-cast, with a stage2 compiler. I think that captures the expected use case: existing ubsan users enabling this new check. > (The numbers won't be too representable, whole stage-1 takes ~40 minutes > here...) Ah I see, I'll run a few builds and take a stab at it, then. Repository: rC Clang https://reviews.llvm.org/D48958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part
vsk added a comment. I have some minor comments but overall I think this is in good shape. It would be great to see some compile-time numbers just to make sure this is tractable. I'm pretty sure -fsanitize=null would fire more often across a codebase than this check, so I don't anticipate a big surprise here. Comment at: lib/CodeGen/CGExprScalar.cpp:222 + + // The stack of currently-visiting Cast expressions. + llvm::SmallVector CastExprStack; It would help to have this comment explain that the stack is used/maintained exclusively by the implicit cast sanitizer. Comment at: lib/CodeGen/CGExprScalar.cpp:228 + +// We only care if we are to use this data. +bool Enabled() { Could you make this comment more specific -- maybe by explaining that for efficiency reasons, the cast expr stack is only maintained when a sanitizer check is enabled? Comment at: lib/CodeGen/CGExprScalar.cpp:234 + public: +CastExprStackGuard(ScalarExprEmitter *SEE, CastExpr *CE) : SEE(SEE) { + if (!Enabled()) I think if you were to use references instead of pointers here, the code would be a bit clearer, and you wouldn't need to assert that CE is non-null. Comment at: lib/CodeGen/CGExprScalar.cpp:351 +ScalarConversionOpts() +: TreatBooleanAsSigned(false), + EmitImplicitIntegerTruncationChecks(false) {} Why not use default member initializers here (e.g, "bool a = false")? Comment at: lib/CodeGen/CGExprScalar.cpp:988 + return Child == PreviousCast; +})) + return false; The none_of call could safely be replaced by `Cast->getSubExpr() != PreviousCast`, I think. Repository: rC Clang https://reviews.llvm.org/D48958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls
NoQ added a comment. > A call to `Derived::Derived()` previously emitted no warnings. However, with > these changes, a warning is emitted for `Base::a`. Yep, a heuristic to skip implicit constructor declarations during analysis and make the first explicit constructor responsible for initializing its bases/fields that have been constructed via autogenerated constructors seems reasonable. With that i guess you'll still have to deep-scan fields and bases, which prevents us from simplifying our code. I'll think about that a bit more; it might be worth it to track such deferred subregions in a state trait and drain it whenever we pop back to an explicit constructor. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689 +Optional OtherObject = +getObjectVal(OtherCtor, Context); +if (!OtherObject) + continue; + +// If the CurrentObject is a field of OtherObject, it will be analyzed +// during the analysis of OtherObject. It seems safer to look at `CXXConstructExpr::getConstructionKind()`. Taking a `LazyCompoundVal` and converting it back to a region is definitely a bad idea because the region within `LazyCompoundVal` is completely immaterial and carries no meaning for the value represented by this `SVal`; it's not necessarily the region you're looking for. https://reviews.llvm.org/D48436 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674 + const LocationContext *LC = Context.getLocationContext(); + while ((LC = LC->getParent())) { + george.karpenkov wrote: > nit: could we have `while (LC)` followed by `LC = LC->getParent()` ? Do you > intentionally skip the first location context? I guess the predicate we're checking is trivially true for the current location context. https://reviews.llvm.org/D48436 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336937 - [Driver] Conform warn_drv_object_size_disabled_O0 to DefaultWarnNoError
Author: vedantk Date: Thu Jul 12 12:53:15 2018 New Revision: 336937 URL: http://llvm.org/viewvc/llvm-project?rev=336937&view=rev Log: [Driver] Conform warn_drv_object_size_disabled_O0 to DefaultWarnNoError This diagnostic triggers when -fsanitize=object-size is explicitly specified but will be a no-op (i.e, at -O0). This diagnostic should not fail a -Werror build because it's just an explanatory note to the user. It's not always actionable. For example, a user may not be able to simply disable object-size, because they want it enabled in optimized builds. rdar://42128447 Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/test/Driver/fsanitize-object-size.c Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=336937&r1=336936&r2=336937&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Jul 12 12:53:15 2018 @@ -273,7 +273,7 @@ def warn_drv_disabling_vptr_no_rtti_defa InGroup; def warn_drv_object_size_disabled_O0 : Warning< "the object size sanitizer has no effect at -O0, but is explicitly enabled: %0">, - InGroup; + InGroup, DefaultWarnNoWerror; def note_drv_command_failed_diag_msg : Note< "diagnostic msg: %0">; Modified: cfe/trunk/test/Driver/fsanitize-object-size.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize-object-size.c?rev=336937&r1=336936&r2=336937&view=diff == --- cfe/trunk/test/Driver/fsanitize-object-size.c (original) +++ cfe/trunk/test/Driver/fsanitize-object-size.c Thu Jul 12 12:53:15 2018 @@ -2,8 +2,9 @@ // // RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE // RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size %s -O0 -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE -// RUN: %clang -target x86_64-linux-gnu -fsanitize=null,object-size %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-OSIZE -// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-OSIZE-NO-WARNING +// RUN: %clang -target x86_64-linux-gnu -fsanitize=null,object-size %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE +// RUN: %clang -target x86_64-linux-gnu -Werror -fsanitize=null,object-size %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE +// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE-NO-WARNING // Check that the object size check is enabled at other optimization levels. // ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)
Wawha updated this revision to Diff 155254. Wawha retitled this revision from "[Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call" to "[Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)". Wawha added a comment. Here a new patch, which is very similar to the second patch. I just remove the extra line parsing which was no necessary since a new tag TT_LambdaLSquare was added. I'm agree with klimek, my third patch was quite weird and after some test contain an error in the code and unittest (inside the first test I added...) This 4th version is far better while it's very similar to the 2nd patch. Repository: rC Clang https://reviews.llvm.org/D44609 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -10461,6 +10461,7 @@ CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock); CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch); CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse); + CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody); CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces); CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction); CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord); @@ -11403,6 +11404,48 @@ "> {\n" " //\n" "});"); + + // Check option "BraceWrapping.BeforeLambdaBody" + FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle(); + LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom; + LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true; + verifyFormat("FunctionWithOneParam(\n" + "[]()\n" + "{\n" + " // A cool function...\n" + " return 43;\n" + "});", + LLVMWithBeforeLambdaBody); + verifyFormat("FunctionWithTwoParams(\n" + "[]()\n" + "{\n" + " // A cool function...\n" + " return 43;\n" + "},\n" + "87);", + LLVMWithBeforeLambdaBody); + verifyFormat("FunctionWithOneNestedLambdas(\n" + "[]()\n" + "{\n" + " return 17;\n" + "});", + LLVMWithBeforeLambdaBody); + verifyFormat("TwoNestedLambdas(\n" + "[]()\n" + "{\n" + " return Call(\n" + " []()\n" + " {\n" + "return 17;\n" + " });\n" + "});", + LLVMWithBeforeLambdaBody); + verifyFormat("auto array = {[]()\n" + " {\n" + "return 43;\n" + " },\n" + " MyFunctor};", + LLVMWithBeforeLambdaBody); } TEST_F(FormatTest, EmptyLinesInLambdas) { Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -1404,6 +1404,7 @@ return true; } } + FormatTok->Type = TT_LambdaLBrace; LSquare.Type = TT_LambdaLSquare; parseChildBlock(); return true; Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -1108,11 +1108,11 @@ // Reset token type in case we have already looked at it and then // recovered from an error (e.g. failure to find the matching >). -if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro, - TT_FunctionLBrace, TT_ImplicitStringLiteral, - TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow, - TT_OverloadedOperator, TT_RegexLiteral, - TT_TemplateString, TT_ObjCStringLiteral)) +if (!CurrentToken->isOneOf( +TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro, +TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace, +TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator, +TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral)) CurrentToken->Type = TT_Unknown; CurrentToken->Role.reset(); CurrentToken->MatchingParen = nullptr; @@ -2896,7 +2896,9 @@ if (Right.is(TT_InlineASMBrace)) return Right.HasUnesc
[PATCH] D48845: [Sema] Add fixit for unused lambda captures
aaron.ballman added inline comments. Comment at: include/clang/Sema/DeclSpec.h:2552-2553 ParsedType InitCaptureType; +SourceLocation LocStart; +SourceLocation LocEnd; + How does `LocStart` relate to the existing source location `Loc`? I think this should have a more descriptive name of what location is involved. Comment at: lib/Sema/SemaLambda.cpp:827 Var->getType(), Var->getInit()); + return Field; Spurious whitespace change can be removed. Comment at: lib/Sema/SemaLambda.cpp:1553 + SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I]; + if (CaptureRange.isInvalid()) { +CaptureRange = SourceRange(From.getLocation()); Can elide braces. Comment at: test/FixIt/fixit-unused-lambda-capture.cpp:64-66 +[*this] {}; +// CHECK: [] {}; +[*this,i] { return i; }; I'd like to see some tests where the `this` and `*this` captures are not removed by the fix. Repository: rC Clang https://reviews.llvm.org/D48845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49188: [OpenMP] Initialize data sharing stack for SPMD case
ABataev added inline comments. Comment at: test/OpenMP/nvptx_data_sharing.cpp:33 // CK1: call void @llvm.nvvm.barrier0() -// CK1: call void @__kmpc_data_sharing_init_stack It is better to check that this call is not emitted, like this `CK1-NOT: call void @__kmpc_data_sharing_init_stack` Comment at: test/OpenMP/nvptx_data_sharing_spmd.cpp:1-24 +// Test device global memory data sharing initialization codegen for spmd. +///==/// + +// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc +// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CK1 + +// expected-no-diagnostics I think we already have some tests with SPMD construct, you can just modify the existing tests instead of adding another one (e.g. `nvptx_target_parallel_codegen.cpp`) Repository: rC Clang https://reviews.llvm.org/D49188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49265: [Tooling] Add operator== to CompileCommand
simark created this revision. Herald added subscribers: cfe-commits, ioeric, ilya-biryukov. It does the obvious thing of comparing all fields. This will be needed for a clangd patch I have in the pipeline. Repository: rC Clang https://reviews.llvm.org/D49265 Files: include/clang/Tooling/CompilationDatabase.h Index: include/clang/Tooling/CompilationDatabase.h === --- include/clang/Tooling/CompilationDatabase.h +++ include/clang/Tooling/CompilationDatabase.h @@ -59,6 +59,11 @@ /// The output file associated with the command. std::string Output; + + bool operator==(const CompileCommand &RHS) const { +return Directory == RHS.Directory && Filename == RHS.Filename && + CommandLine == RHS.CommandLine && Output == RHS.Output; + } }; /// Interface for compilation databases. Index: include/clang/Tooling/CompilationDatabase.h === --- include/clang/Tooling/CompilationDatabase.h +++ include/clang/Tooling/CompilationDatabase.h @@ -59,6 +59,11 @@ /// The output file associated with the command. std::string Output; + + bool operator==(const CompileCommand &RHS) const { +return Directory == RHS.Directory && Filename == RHS.Filename && + CommandLine == RHS.CommandLine && Output == RHS.Output; + } }; /// Interface for compilation databases. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager
NoQ added a comment. I'd also rather stick to integer arithmetic and avoid using floats even in intermediate calculations. It'd be hard to make sure that no rounding errors kick in if we use floats. https://reviews.llvm.org/D49074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49267: [clangd] Watch for changes in compile_commands.json
simark created this revision. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. This patch adds support for watching for changes to compile_commands.json, and reparsing files if needed. The watching is done using the "workspace/didChangeWatchedFiles" notification, so the actual file watching is the frontend's problem. To keep things simple, we react to any change involving a file called "compile_commands.json". The chosen strategy tries to avoid useless reparses. We don't want to reparse a file if its compile commands don't change. So when we get a change notification, we: 1. Save the compile commands of all open files on the side. 2. Clear everything we know about compilation databases and compilation commands. 3. Query the compile commands again for all open files (which will go read the possibly updated compile commands). If the commands are different than the saved ones, we reparse the file. I updated the vscode-clangd extension. If you don't feel inspired, you can use this small .cpp to test it: #ifdef MACRO #warning "MACRO is defined" #else #warning "MACRO is not defined" #endif int main() {} and this compile_commands.json: [{ "directory": ".", "file": "test.cpp", "arguments": ["g++", "-c", "test.cpp", "-DMACRO=2"] }] Adding and removing the "-DMACRO=2" argument, you should see a different Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49267 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/clients/clangd-vscode/src/extension.ts Index: clangd/clients/clangd-vscode/src/extension.ts === --- clangd/clients/clangd-vscode/src/extension.ts +++ clangd/clients/clangd-vscode/src/extension.ts @@ -30,25 +30,32 @@ } const serverOptions: vscodelc.ServerOptions = clangd; -const filePattern: string = '**/*.{' + -['cpp', 'c', 'cc', 'cxx', 'c++', 'm', 'mm', 'h', 'hh', 'hpp', 'hxx', 'inc'].join() + '}'; +const sourceFilePattern: string = '**/*.{' + [ + 'cpp', 'c', 'cc', 'cxx', 'c++', 'm', 'mm', 'h', 'hh', 'hpp', 'hxx', 'inc' +].join() + '}'; +const compileCommandsFilePattern: string = '**/compile_commands.json'; const clientOptions: vscodelc.LanguageClientOptions = { -// Register the server for C/C++ files -documentSelector: [{ scheme: 'file', pattern: filePattern }], -synchronize: !syncFileEvents ? undefined : { -fileEvents: vscode.workspace.createFileSystemWatcher(filePattern) -}, -// Resolve symlinks for all files provided by clangd. -// This is a workaround for a bazel + clangd issue - bazel produces a symlink tree to build in, -// and when navigating to the included file, clangd passes its path inside the symlink tree -// rather than its filesystem path. -// FIXME: remove this once clangd knows enough about bazel to resolve the -// symlinks where needed (or if this causes problems for other workflows). -uriConverters: { -code2Protocol: (value: vscode.Uri) => value.toString(), -protocol2Code: (value: string) => -vscode.Uri.file(realpathSync(vscode.Uri.parse(value).fsPath)) -} + // Register the server for C/C++ files + documentSelector : [ {scheme : 'file', pattern : sourceFilePattern} ], + synchronize : !syncFileEvents ? undefined : { +fileEvents : [ + vscode.workspace.createFileSystemWatcher(sourceFilePattern), + vscode.workspace.createFileSystemWatcher(compileCommandsFilePattern), +] + }, + // Resolve symlinks for all files provided by clangd. + // This is a workaround for a bazel + clangd issue - bazel produces a + // symlink tree to build in, + // and when navigating to the included file, clangd passes its path inside + // the symlink tree + // rather than its filesystem path. + // FIXME: remove this once clangd knows enough about bazel to resolve the + // symlinks where needed (or if this causes problems for other workflows). + uriConverters : { +code2Protocol : (value: vscode.Uri) => value.toString(), +protocol2Code : (value: string) => +vscode.Uri.file(realpathSync(vscode.Uri.parse(value).fsPath)) + } }; const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions); Index: clangd/ProtocolHandlers.h === --- clangd/ProtocolHandlers.h +++ clangd/ProtocolHandlers.h @@ -48,7 +48,7 @@ virtual void onSignatureHelp(TextDocumentPositionParams &Params) = 0; virtual void onGoToDefinition(TextDocumentPositionParams &Params) = 0; virtual void onSwitchSourceHeader
r336946 - PR38141: check whether noexcept-specifications are equivalent in redeclarations
Author: rsmith Date: Thu Jul 12 14:11:25 2018 New Revision: 336946 URL: http://llvm.org/viewvc/llvm-project?rev=336946&view=rev Log: PR38141: check whether noexcept-specifications are equivalent in redeclarations Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp cfe/trunk/test/CXX/except/except.spec/p3.cpp cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=336946&r1=336945&r2=336946&view=diff == --- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original) +++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Thu Jul 12 14:11:25 2018 @@ -530,10 +530,16 @@ static bool CheckEquivalentExceptionSpec } } - // FIXME: We treat dependent noexcept specifications as compatible even if - // their expressions are not equivalent. - if (OldEST == EST_DependentNoexcept && NewEST == EST_DependentNoexcept) -return false; + // C++14 [except.spec]p3: + // Two exception-specifications are compatible if [...] both have the form + // noexcept(constant-expression) and the constant-expressions are equivalent + if (OldEST == EST_DependentNoexcept && NewEST == EST_DependentNoexcept) { +llvm::FoldingSetNodeID OldFSN, NewFSN; +Old->getNoexceptExpr()->Profile(OldFSN, S.Context, true); +New->getNoexceptExpr()->Profile(NewFSN, S.Context, true); +if (OldFSN == NewFSN) + return false; + } // Dynamic exception specifications with the same set of adjusted types // are compatible. Modified: cfe/trunk/test/CXX/except/except.spec/p3.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p3.cpp?rev=336946&r1=336945&r2=336946&view=diff == --- cfe/trunk/test/CXX/except/except.spec/p3.cpp (original) +++ cfe/trunk/test/CXX/except/except.spec/p3.cpp Thu Jul 12 14:11:25 2018 @@ -104,3 +104,13 @@ void* operator new(mysize_t); void* operator new[](mysize_t); void* operator new[](mysize_t) throw(std::bad_alloc); +template void equivalent() noexcept (X); +template void equivalent() noexcept (X); + +template void not_equivalent() noexcept (X); // expected-note {{previous}} +template void not_equivalent() noexcept (Y); // expected-error {{does not match}} + +template void missing() noexcept (X); // expected-note {{previous}} +// FIXME: The missing exception specification that we report here doesn't make +// sense in the context of this declaration. +template void missing(); // expected-error {{missing exception specification 'noexcept(X)'}} Modified: cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp?rev=336946&r1=336945&r2=336946&view=diff == --- cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp (original) +++ cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp Thu Jul 12 14:11:25 2018 @@ -10,7 +10,7 @@ template void redecl1() noex template void redecl1() noexcept(noexcept(T())) {} // expected-error {{redefinition}} template void redecl2() noexcept(A); // expected-note {{previous}} -template void redecl2() noexcept(B); // expected-error {{conflicting types}} +template void redecl2() noexcept(B); // expected-error {{does not match previous}} // These have the same canonical type, but are still different. template void redecl3() throw(A); // expected-note {{previous}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336947 - Support linking static PIE binaries on NetBSD
Author: joerg Date: Thu Jul 12 14:21:29 2018 New Revision: 336947 URL: http://llvm.org/viewvc/llvm-project?rev=336947&view=rev Log: Support linking static PIE binaries on NetBSD Modified: cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp cfe/trunk/test/Driver/netbsd.c Modified: cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp?rev=336947&r1=336946&r2=336947&view=diff == --- cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp Thu Jul 12 14:21:29 2018 @@ -122,6 +122,10 @@ void netbsd::Linker::ConstructJob(Compil CmdArgs.push_back("--eh-frame-hdr"); if (Args.hasArg(options::OPT_static)) { CmdArgs.push_back("-Bstatic"); +if (Args.hasArg(options::OPT_pie)) { + Args.AddAllArgs(CmdArgs, options::OPT_pie); + CmdArgs.push_back("--no-dynamic-linker"); +} } else { if (Args.hasArg(options::OPT_rdynamic)) CmdArgs.push_back("-export-dynamic"); Modified: cfe/trunk/test/Driver/netbsd.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/netbsd.c?rev=336947&r1=336946&r2=336947&view=diff == --- cfe/trunk/test/Driver/netbsd.c (original) +++ cfe/trunk/test/Driver/netbsd.c Thu Jul 12 14:21:29 2018 @@ -5,6 +5,9 @@ // RUN: -pie --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \ // RUN: | FileCheck -check-prefix=PIE %s // RUN: %clang -no-canonical-prefixes -target x86_64--netbsd \ +// RUN: -static -pie --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=STATIC-PIE %s +// RUN: %clang -no-canonical-prefixes -target x86_64--netbsd \ // RUN: -shared --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \ // RUN: | FileCheck -check-prefix=SHARED %s @@ -139,6 +142,16 @@ // STATIC: "{{.*}}/usr/lib{{/|}}crti.o" "{{.*}}/usr/lib{{/|}}crtbegin.o" // STATIC: "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*}}/usr/lib{{/|}}crtn.o" +// STATIC-PIE: ld{{.*}}" "--eh-frame-hdr" +// STATIC-PIE-NOT: "-dynamic-linker" "/libexec/ld.elf_so" +// STATIC-PIE-NOT: "-Bshareable" +// STATIC-PIE: "-pie" +// STATIC-PIE-NOT: "-dynamic-linker" "/libexec/ld.elf_so" +// STATIC-PIE-NOT: "-Bshareable" +// STATIC-PIE: "{{.*}}/usr/lib{{/|}}crt0.o" +// STATIC-PIE: "{{.*}}/usr/lib{{/|}}crti.o" "{{.*}}/usr/lib{{/|}}crtbeginS.o" +// STATIC-PIE: "{{.*}}/usr/lib{{/|}}crtendS.o" "{{.*}}/usr/lib{{/|}}crtn.o" + // SHARED: ld{{.*}}" "--eh-frame-hdr" // SHARED-NOT: "-pie" // SHARED-NOT: "-dynamic-linker" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added inline comments. Comment at: libcxx/include/__hash_table:2165 +#if _LIBCPP_STD_VER > 14 +template +template ldionne wrote: > When a function is declared with a visibility macro > (`_LIBCPP_INLINE_VISIBILITY` in this case), I think it is customary in libc++ > to repeat the visibility macro on the definition as well. If that's correct, > apply here and to other similar functions below, and also in `__tree`. I can't say I know libc++'s policy here either, but it couldn't hurt, so I added the attributes to the new patch. Maybe @EricWF or @mclow.lists could weigh in here? Comment at: libcxx/include/__hash_table:2212 +allocator_type __alloc(__node_alloc()); +return _NodeHandle(remove(__p).release(), __alloc); +} ldionne wrote: > Just a quick Q: am I mistaken or `remove()` is not part of the > `unordered_map` API? We're not naming it `__remove()` just because we already > have other functions called `remove()`, so any macro redefining `remove` > would already break things -- is that the idea? Yep, I think that was the thinking. IMO it would be nice to just call it `__remove` to avoid any confusion. Comment at: libcxx/include/__node_handle:36 +template +class __node_handle_base +{ ldionne wrote: > I'd like to suggest a different approach for implementing the node handles > for sets and maps. I believe this would simplify things slightly: > > ``` > template class > _MapOrSetSpecifics> > class __basic_node_handle > : public _MapOrSetSpecifics<_NodeType, __basic_node_handle<_NodeType, > _Alloc, _MapOrSetSpecifics>> > { > // same as right now > }; > ``` > > Then, you can get the two different node handles by simply providing the > methods in the base class: > > ``` > template > struct __map_node_handle_specifics { > using key_type = typename _NodeType::__node_value_type::key_type; > using mapped_type = typename _NodeType::__node_value_type::mapped_type; > > _LIBCPP_INLINE_VISIBILITY > key_type& key() const > { > return static_cast<_Derived > const*>(this)->__ptr_->__value_.__ref().first; > } > > _LIBCPP_INLINE_VISIBILITY > mapped_type& mapped() const > { > return static_cast<_Derived > const*>(this)->__ptr_->__value_.__ref().second; > } > }; > > template > struct __set_node_handle_specifics { > using value_type = typename _NodeType::__node_value_type; > > _LIBCPP_INLINE_VISIBILITY > value_type& value() const > { > return static_cast<_Derived const*>(this)->__ptr_->__value_; > } > }; > > template > using __map_node_handle = __basic_node_handle<_NodeType, _Alloc, > __map_node_handle_specifics>; > > template > using __set_node_handle = __basic_node_handle<_NodeType, _Alloc, > __set_node_handle_specifics>; > ``` > > This is a classic application of the CRTP. I believe it would reduce the > amount of boilerplate currently required for special member functions. It's > possible that you thought of this and realized it didn't work for a reason > I'm missing right now, too, but the idea seems to work on the surface: > https://wandbox.org/permlink/nMaKEg43PVJpA0ld. > > You don't have to do this, but please consider it if you think it would > simplify the code. > Oh, neat! That's a great idea, I didn't even consider CRTP here for some reason. Makes this simpler though. I added this to the new patch. Comment at: libcxx/include/map:915 + +__base& __get_tree() { return __tree_; } + ldionne wrote: > I believe this function should be marked with `_LIBCPP_INLINE_VISIBILITY` -- > even though in practice the compiler will probably always inline it. > > Also, you don't seem to be using that function at all in the diff -- is that > right? > > So please either mark with `_LIBCPP_INLINE_VISIBILITY` and use it, or remove > it altogether. This comment applies to all 8 containers to which similar > methods were added. Oh, sorry! This was used in the implementation of `merge`, which I moved to a follow-up. I'll sink it down there and add the visibility attribute. Comment at: libcxx/include/set:399 +template +class multiset; ldionne wrote: > Is this forward declaration necessary? No, nice catch! This was also supposed to be a part of the follow-up, I'll move it there. Comment at: libcxx/test/std/containers/unord/unord.map/unord.map.modifiers/insert_extract.pass.cpp:14 + +// class unordered_map + ldionne wrote: > Please be more specific about which functions you're testing. I find it > useful to look at those comments to quickly see what exact signatures are > being tested when I'm looking for tests that exercice some specific piece of > code. > > Actually, perhaps you should extract those tests into multiple files: > `unord.map/unord.map.modifiers/ins
[PATCH] D49267: [clangd] Watch for changes in compile_commands.json
simark added a comment. Note, https://reviews.llvm.org/D49265 in clang is a prerequisite. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49267: [clangd] Watch for changes in compile_commands.json
simark updated this revision to Diff 155284. simark added a comment. Remove unintended changes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49267 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/clients/clangd-vscode/src/extension.ts Index: clangd/clients/clangd-vscode/src/extension.ts === --- clangd/clients/clangd-vscode/src/extension.ts +++ clangd/clients/clangd-vscode/src/extension.ts @@ -30,14 +30,18 @@ } const serverOptions: vscodelc.ServerOptions = clangd; -const filePattern: string = '**/*.{' + +const sourceFilePattern: string = '**/*.{' + ['cpp', 'c', 'cc', 'cxx', 'c++', 'm', 'mm', 'h', 'hh', 'hpp', 'hxx', 'inc'].join() + '}'; +const compileCommandsFilePattern: string = '**/compile_commands.json'; const clientOptions: vscodelc.LanguageClientOptions = { // Register the server for C/C++ files -documentSelector: [{ scheme: 'file', pattern: filePattern }], +documentSelector: [{ scheme: 'file', pattern: sourceFilePattern }], synchronize: !syncFileEvents ? undefined : { -fileEvents: vscode.workspace.createFileSystemWatcher(filePattern) -}, +fileEvents : [ + vscode.workspace.createFileSystemWatcher(sourceFilePattern), + vscode.workspace.createFileSystemWatcher(compileCommandsFilePattern), +] + }, // Resolve symlinks for all files provided by clangd. // This is a workaround for a bazel + clangd issue - bazel produces a symlink tree to build in, // and when navigating to the included file, clangd passes its path inside the symlink tree Index: clangd/ProtocolHandlers.h === --- clangd/ProtocolHandlers.h +++ clangd/ProtocolHandlers.h @@ -48,7 +48,7 @@ virtual void onSignatureHelp(TextDocumentPositionParams &Params) = 0; virtual void onGoToDefinition(TextDocumentPositionParams &Params) = 0; virtual void onSwitchSourceHeader(TextDocumentIdentifier &Params) = 0; - virtual void onFileEvent(DidChangeWatchedFilesParams &Params) = 0; + virtual void onDidChangeWatchedFiles(DidChangeWatchedFilesParams &Params) = 0; virtual void onCommand(ExecuteCommandParams &Params) = 0; virtual void onWorkspaceSymbol(WorkspaceSymbolParams &Params) = 0; virtual void onRename(RenameParams &Parames) = 0; Index: clangd/ProtocolHandlers.cpp === --- clangd/ProtocolHandlers.cpp +++ clangd/ProtocolHandlers.cpp @@ -68,7 +68,8 @@ Register("textDocument/rename", &ProtocolCallbacks::onRename); Register("textDocument/hover", &ProtocolCallbacks::onHover); Register("textDocument/documentSymbol", &ProtocolCallbacks::onDocumentSymbol); - Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent); + Register("workspace/didChangeWatchedFiles", + &ProtocolCallbacks::onDidChangeWatchedFiles); Register("workspace/executeCommand", &ProtocolCallbacks::onCommand); Register("textDocument/documentHighlight", &ProtocolCallbacks::onDocumentHighlight); Index: clangd/GlobalCompilationDatabase.h === --- clangd/GlobalCompilationDatabase.h +++ clangd/GlobalCompilationDatabase.h @@ -68,6 +68,9 @@ /// Sets the extra flags that should be added to a file. void setExtraFlagsForFile(PathRef File, std::vector ExtraFlags); + /// Forget the compilation flags cached in this object. + void clear(); + private: tooling::CompilationDatabase *getCDBForFile(PathRef File) const; tooling::CompilationDatabase *getCDBInDirLocked(PathRef File) const; Index: clangd/GlobalCompilationDatabase.cpp === --- clangd/GlobalCompilationDatabase.cpp +++ clangd/GlobalCompilationDatabase.cpp @@ -66,6 +66,11 @@ CompilationDatabases.clear(); } +void DirectoryBasedGlobalCompilationDatabase::clear() { + std::lock_guard Lock(Mutex); + CompilationDatabases.clear(); +} + void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile( PathRef File, std::vector ExtraFlags) { std::lock_guard Lock(Mutex); Index: clangd/ClangdLSPServer.h === --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -69,19 +69,45 @@ void onGoToDefinition(TextDocumentPositionParams &Params) override; void onSwitchSourceHeader(TextDocumentIdentifier &Params) override; void onDocumentHighlight(TextDocumentPositionParams &Params) override; - void onFileEvent(DidChangeWatchedFilesParams &Params) override; + void onDidChangeWatchedFiles(DidChangeWatchedFilesParams &Params) override; void onCommand(
[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization
jfb updated this revision to Diff 155285. jfb marked an inline comment as done. jfb added a comment. - Use Address as suggested in review. Repository: rC Clang https://reviews.llvm.org/D49209 Files: lib/CodeGen/CGBuilder.h lib/CodeGen/CGDecl.cpp test/CodeGen/init.c test/CodeGenOpenCL/partial_initializer.cl Index: test/CodeGenOpenCL/partial_initializer.cl === --- test/CodeGenOpenCL/partial_initializer.cl +++ test/CodeGenOpenCL/partial_initializer.cl @@ -38,11 +38,11 @@ // CHECK: %[[v0:.*]] = bitcast [6 x [6 x float]]* %A to i8* // CHECK: call void @llvm.memset.p0i8.i32(i8* align 4 %[[v0]], i8 0, i32 144, i1 false) // CHECK: %[[v1:.*]] = bitcast i8* %[[v0]] to [6 x [6 x float]]* - // CHECK: %[[v2:.*]] = getelementptr [6 x [6 x float]], [6 x [6 x float]]* %[[v1]], i32 0, i32 0 - // CHECK: %[[v3:.*]] = getelementptr [6 x float], [6 x float]* %[[v2]], i32 0, i32 0 - // CHECK: store float 1.00e+00, float* %[[v3]] - // CHECK: %[[v4:.*]] = getelementptr [6 x float], [6 x float]* %[[v2]], i32 0, i32 1 - // CHECK: store float 2.00e+00, float* %[[v4]] + // CHECK: %[[v2:.*]] = getelementptr inbounds [6 x [6 x float]], [6 x [6 x float]]* %[[v1]], i32 0, i32 0 + // CHECK: %[[v3:.*]] = getelementptr inbounds [6 x float], [6 x float]* %[[v2]], i32 0, i32 0 + // CHECK: store float 1.00e+00, float* %[[v3]], align 4 + // CHECK: %[[v4:.*]] = getelementptr inbounds [6 x float], [6 x float]* %[[v2]], i32 0, i32 1 + // CHECK: store float 2.00e+00, float* %[[v4]], align 4 float A[6][6] = {1.0f, 2.0f}; // CHECK: %[[v5:.*]] = bitcast %struct.StrucTy* %S to i8* Index: test/CodeGen/init.c === --- test/CodeGen/init.c +++ test/CodeGen/init.c @@ -86,25 +86,25 @@ char test8(int X) { char str[10] = "abc"; // tail should be memset. return str[X]; -// CHECK: @test8( -// CHECK: call void @llvm.memset -// CHECK: store i8 97 -// CHECK: store i8 98 -// CHECK: store i8 99 -// CHECK-NOT: getelementptr -// CHECK: load + // CHECK-LABEL: @test8( + // CHECK: call void @llvm.memset + // CHECK: store i8 97, i8* %{{[0-9]*}}, align 1 + // CHECK: store i8 98, i8* %{{[0-9]*}}, align 1 + // CHECK: store i8 99, i8* %{{[0-9]*}}, align 1 + // CHECK-NOT: getelementptr + // CHECK: load } void bar(void*); // PR279 -int test9(int X) { +void test9(int X) { int Arr[100] = { X }; // Should use memset bar(Arr); -// CHECK: @test9 -// CHECK: call void @llvm.memset -// CHECK-NOT: store i32 0 -// CHECK: call void @bar + // CHECK-LABEL: @test9( + // CHECK: call void @llvm.memset + // CHECK-NOT: store i32 0 + // CHECK: call void @bar } struct a { @@ -115,11 +115,11 @@ struct a a,b,c,d,e,f,g; }; -int test10(int X) { +void test10(int X) { struct b S = { .a.a = X, .d.e = X, .f.e = 0, .f.f = 0, .f.p = 0 }; bar(&S); - // CHECK: @test10 + // CHECK-LABEL: @test10( // CHECK: call void @llvm.memset // CHECK-NOT: store i32 0 // CHECK: call void @bar @@ -132,11 +132,11 @@ }; void test11(struct test11S *P) { *P = (struct test11S) { .A = { [0 ... 3] = 4 } }; - // CHECK: @test11 - // CHECK: store i32 4 - // CHECK: store i32 4 - // CHECK: store i32 4 - // CHECK: store i32 4 + // CHECK-LABEL: @test11( + // CHECK: store i32 4, i32* %{{.*}}, align 4 + // CHECK: store i32 4, i32* %{{.*}}, align 4 + // CHECK: store i32 4, i32* %{{.*}}, align 4 + // CHECK: store i32 4, i32* %{{.*}}, align 4 // CHECK: ret void } @@ -151,11 +151,11 @@ void test13(int x) { struct X { int a; int b : 10; int c; }; struct X y = {.c = x}; - // CHECK: @test13 + // CHECK-LABEL: @test13( // CHECK: and i16 {{.*}}, -1024 } -// CHECK-LABEL: @PR20473 +// CHECK-LABEL: @PR20473( void PR20473() { // CHECK: memcpy{{.*}}getelementptr inbounds ([2 x i8], [2 x i8]* @ bar((char[2]) {""}); @@ -168,9 +168,9 @@ struct S14 { int a[16]; }; void test14(struct S14 *s14) { -// CHECK-LABEL: @test14 -// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 4 {{.*}} [[INIT14]] {{.*}}, i32 64, i1 false) -// CHECK-NOT: store -// CHECK: ret void + // CHECK-LABEL: @test14( + // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 4 {{.*}} [[INIT14]] {{.*}}, i32 64, i1 false) + // CHECK-NOT: store + // CHECK: ret void *s14 = (struct S14) { { [5 ... 11] = 17 } }; } Index: lib/CodeGen/CGDecl.cpp === --- lib/CodeGen/CGDecl.cpp +++ lib/CodeGen/CGDecl.cpp @@ -888,15 +888,18 @@ /// emitStoresForInitAfterMemset - For inits that /// canEmitInitWithFewStoresAfterMemset returned true for, emit the scalar /// stores that would be required. -static void emitStoresForInitAfterMemset(llvm::Constant *Init, llvm::Value *Loc, - bool isVolatile, CGBuilderTy &Builder) { +static void emitStoresForInitAfterMemset(CodeG
[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization
jfb added a comment. I updated the patch to use `Address`, and also use `inbounds`. This was a bit tricky because of the GEPs. I also updated tests which run into this codegen. Only the following tests actually hit this code, and not all check these stores: Clang :: CodeGen/2004-03-09-LargeArrayInitializers.c Clang :: CodeGen/indirect-goto.c Clang :: CodeGen/init.c Clang :: CodeGenCXX/float128-declarations.cpp Clang :: CodeGenObjC/messages-2.m Clang :: CodeGenObjC/messages.m Clang :: CodeGenOpenCL/address-space-constant-initializers.cl Clang :: CodeGenOpenCL/partial_initializer.cl Clang :: CodeGenOpenCL/private-array-initialization.cl Repository: rC Clang https://reviews.llvm.org/D49209 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization
jfb updated this revision to Diff 155287. jfb added a comment. - Fix silly naming and lookup. Repository: rC Clang https://reviews.llvm.org/D49209 Files: lib/CodeGen/CGBuilder.h lib/CodeGen/CGDecl.cpp test/CodeGen/init.c test/CodeGenOpenCL/partial_initializer.cl Index: test/CodeGenOpenCL/partial_initializer.cl === --- test/CodeGenOpenCL/partial_initializer.cl +++ test/CodeGenOpenCL/partial_initializer.cl @@ -38,11 +38,11 @@ // CHECK: %[[v0:.*]] = bitcast [6 x [6 x float]]* %A to i8* // CHECK: call void @llvm.memset.p0i8.i32(i8* align 4 %[[v0]], i8 0, i32 144, i1 false) // CHECK: %[[v1:.*]] = bitcast i8* %[[v0]] to [6 x [6 x float]]* - // CHECK: %[[v2:.*]] = getelementptr [6 x [6 x float]], [6 x [6 x float]]* %[[v1]], i32 0, i32 0 - // CHECK: %[[v3:.*]] = getelementptr [6 x float], [6 x float]* %[[v2]], i32 0, i32 0 - // CHECK: store float 1.00e+00, float* %[[v3]] - // CHECK: %[[v4:.*]] = getelementptr [6 x float], [6 x float]* %[[v2]], i32 0, i32 1 - // CHECK: store float 2.00e+00, float* %[[v4]] + // CHECK: %[[v2:.*]] = getelementptr inbounds [6 x [6 x float]], [6 x [6 x float]]* %[[v1]], i32 0, i32 0 + // CHECK: %[[v3:.*]] = getelementptr inbounds [6 x float], [6 x float]* %[[v2]], i32 0, i32 0 + // CHECK: store float 1.00e+00, float* %[[v3]], align 4 + // CHECK: %[[v4:.*]] = getelementptr inbounds [6 x float], [6 x float]* %[[v2]], i32 0, i32 1 + // CHECK: store float 2.00e+00, float* %[[v4]], align 4 float A[6][6] = {1.0f, 2.0f}; // CHECK: %[[v5:.*]] = bitcast %struct.StrucTy* %S to i8* Index: test/CodeGen/init.c === --- test/CodeGen/init.c +++ test/CodeGen/init.c @@ -86,25 +86,25 @@ char test8(int X) { char str[10] = "abc"; // tail should be memset. return str[X]; -// CHECK: @test8( -// CHECK: call void @llvm.memset -// CHECK: store i8 97 -// CHECK: store i8 98 -// CHECK: store i8 99 -// CHECK-NOT: getelementptr -// CHECK: load + // CHECK-LABEL: @test8( + // CHECK: call void @llvm.memset + // CHECK: store i8 97, i8* %{{[0-9]*}}, align 1 + // CHECK: store i8 98, i8* %{{[0-9]*}}, align 1 + // CHECK: store i8 99, i8* %{{[0-9]*}}, align 1 + // CHECK-NOT: getelementptr + // CHECK: load } void bar(void*); // PR279 -int test9(int X) { +void test9(int X) { int Arr[100] = { X }; // Should use memset bar(Arr); -// CHECK: @test9 -// CHECK: call void @llvm.memset -// CHECK-NOT: store i32 0 -// CHECK: call void @bar + // CHECK-LABEL: @test9( + // CHECK: call void @llvm.memset + // CHECK-NOT: store i32 0 + // CHECK: call void @bar } struct a { @@ -115,11 +115,11 @@ struct a a,b,c,d,e,f,g; }; -int test10(int X) { +void test10(int X) { struct b S = { .a.a = X, .d.e = X, .f.e = 0, .f.f = 0, .f.p = 0 }; bar(&S); - // CHECK: @test10 + // CHECK-LABEL: @test10( // CHECK: call void @llvm.memset // CHECK-NOT: store i32 0 // CHECK: call void @bar @@ -132,11 +132,11 @@ }; void test11(struct test11S *P) { *P = (struct test11S) { .A = { [0 ... 3] = 4 } }; - // CHECK: @test11 - // CHECK: store i32 4 - // CHECK: store i32 4 - // CHECK: store i32 4 - // CHECK: store i32 4 + // CHECK-LABEL: @test11( + // CHECK: store i32 4, i32* %{{.*}}, align 4 + // CHECK: store i32 4, i32* %{{.*}}, align 4 + // CHECK: store i32 4, i32* %{{.*}}, align 4 + // CHECK: store i32 4, i32* %{{.*}}, align 4 // CHECK: ret void } @@ -151,11 +151,11 @@ void test13(int x) { struct X { int a; int b : 10; int c; }; struct X y = {.c = x}; - // CHECK: @test13 + // CHECK-LABEL: @test13( // CHECK: and i16 {{.*}}, -1024 } -// CHECK-LABEL: @PR20473 +// CHECK-LABEL: @PR20473( void PR20473() { // CHECK: memcpy{{.*}}getelementptr inbounds ([2 x i8], [2 x i8]* @ bar((char[2]) {""}); @@ -168,9 +168,9 @@ struct S14 { int a[16]; }; void test14(struct S14 *s14) { -// CHECK-LABEL: @test14 -// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 4 {{.*}} [[INIT14]] {{.*}}, i32 64, i1 false) -// CHECK-NOT: store -// CHECK: ret void + // CHECK-LABEL: @test14( + // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 4 {{.*}} [[INIT14]] {{.*}}, i32 64, i1 false) + // CHECK-NOT: store + // CHECK: ret void *s14 = (struct S14) { { [5 ... 11] = 17 } }; } Index: lib/CodeGen/CGDecl.cpp === --- lib/CodeGen/CGDecl.cpp +++ lib/CodeGen/CGDecl.cpp @@ -888,15 +888,18 @@ /// emitStoresForInitAfterMemset - For inits that /// canEmitInitWithFewStoresAfterMemset returned true for, emit the scalar /// stores that would be required. -static void emitStoresForInitAfterMemset(llvm::Constant *Init, llvm::Value *Loc, - bool isVolatile, CGBuilderTy &Builder) { +static void emitStoresForInitAfterMemset(CodeGenModule &CGM, +
[PATCH] D48808: [CodeGen] Emit parallel_loop_access for each loop in the loop stack.
Meinersbur updated this revision to Diff 155288. Meinersbur added a comment. - Hoist I->mayReadOrWriteMemory() condition. Repository: rC Clang https://reviews.llvm.org/D48808 Files: lib/CodeGen/CGLoopInfo.cpp test/CodeGenCXX/pragma-loop-safety-nested.cpp test/CodeGenCXX/pragma-loop-safety-outer.cpp Index: test/CodeGenCXX/pragma-loop-safety-outer.cpp === --- /dev/null +++ test/CodeGenCXX/pragma-loop-safety-outer.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s + +// Verify that the inner access is tagged with a parallel_loop_access +// for the outer loop. +void vectorize_outer_test(int *List, int Length) { +#pragma clang loop vectorize(assume_safety) interleave(disable) unroll(disable) + for (int i = 0; i < Length; i += 2) { +#pragma clang loop unroll(full) +for (int j = 0; j < 2; j += 1) + List[i + j] = (i + j) * 2; + } +} + +// CHECK: %[[MUL:.+]] = mul +// CHECK: store i32 %[[MUL]], i32* %{{.+}}, !llvm.mem.parallel_loop_access ![[OUTER_LOOPID:[0-9]+]] +// CHECK: br label %{{.+}}, !llvm.loop ![[INNER_LOOPID:[0-9]+]] +// CHECK: br label %{{.+}}, !llvm.loop ![[OUTER_LOOPID]] + +// CHECK: ![[OUTER_LOOPID]] = distinct !{![[OUTER_LOOPID]], +// CHECK: ![[INNER_LOOPID]] = distinct !{![[INNER_LOOPID]], Index: test/CodeGenCXX/pragma-loop-safety-nested.cpp === --- /dev/null +++ test/CodeGenCXX/pragma-loop-safety-nested.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s + +// Verify that the inner access is tagged with a parallel_loop_access +// for the inner and outer loop using a list. +void vectorize_nested_test(int *List, int Length) { +#pragma clang loop vectorize(assume_safety) interleave(disable) unroll(disable) + for (int i = 0; i < Length; ++i) { +#pragma clang loop vectorize(assume_safety) interleave(disable) unroll(disable) +for (int j = 0; j < Length; ++j) + List[i * Length + j] = (i + j) * 2; + } +} + +// CHECK: %[[MUL:.+]] = mul +// CHECK: store i32 %[[MUL]], i32* %{{.+}}, !llvm.mem.parallel_loop_access ![[PARALLEL_LIST:[0-9]+]] +// CHECK: br label %{{.+}}, !llvm.loop ![[INNER_LOOPID:[0-9]+]] +// CHECK: br label %{{.+}}, !llvm.loop ![[OUTER_LOOPID:[0-9]+]] + +// CHECK: ![[OUTER_LOOPID]] = distinct !{![[OUTER_LOOPID]], +// CHECK: ![[PARALLEL_LIST]] = !{![[OUTER_LOOPID]], ![[INNER_LOOPID]]} +// CHECK: ![[INNER_LOOPID]] = distinct !{![[INNER_LOOPID]], Index: lib/CodeGen/CGLoopInfo.cpp === --- lib/CodeGen/CGLoopInfo.cpp +++ lib/CodeGen/CGLoopInfo.cpp @@ -300,6 +300,17 @@ return; } - if (L.getAttributes().IsParallel && I->mayReadOrWriteMemory()) -I->setMetadata("llvm.mem.parallel_loop_access", L.getLoopID()); + if (I->mayReadOrWriteMemory()) { +SmallVector ParallelLoopIDs; +for (const LoopInfo &AL : Active) + if (AL.getAttributes().IsParallel) +ParallelLoopIDs.push_back(AL.getLoopID()); + +MDNode *ParallelMD = nullptr; +if (ParallelLoopIDs.size() == 1) + ParallelMD = cast(ParallelLoopIDs[0]); +else if (ParallelLoopIDs.size() >= 2) + ParallelMD = MDNode::get(I->getContext(), ParallelLoopIDs); +I->setMetadata("llvm.mem.parallel_loop_access", ParallelMD); + } } Index: test/CodeGenCXX/pragma-loop-safety-outer.cpp === --- /dev/null +++ test/CodeGenCXX/pragma-loop-safety-outer.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s + +// Verify that the inner access is tagged with a parallel_loop_access +// for the outer loop. +void vectorize_outer_test(int *List, int Length) { +#pragma clang loop vectorize(assume_safety) interleave(disable) unroll(disable) + for (int i = 0; i < Length; i += 2) { +#pragma clang loop unroll(full) +for (int j = 0; j < 2; j += 1) + List[i + j] = (i + j) * 2; + } +} + +// CHECK: %[[MUL:.+]] = mul +// CHECK: store i32 %[[MUL]], i32* %{{.+}}, !llvm.mem.parallel_loop_access ![[OUTER_LOOPID:[0-9]+]] +// CHECK: br label %{{.+}}, !llvm.loop ![[INNER_LOOPID:[0-9]+]] +// CHECK: br label %{{.+}}, !llvm.loop ![[OUTER_LOOPID]] + +// CHECK: ![[OUTER_LOOPID]] = distinct !{![[OUTER_LOOPID]], +// CHECK: ![[INNER_LOOPID]] = distinct !{![[INNER_LOOPID]], Index: test/CodeGenCXX/pragma-loop-safety-nested.cpp === --- /dev/null +++ test/CodeGenCXX/pragma-loop-safety-nested.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s + +// Verify that the inner access is tagged with a parallel_loop_access +// for the inner and outer loop using a list. +void vectorize_nested_test(int *List, int Length) { +#pragma clang loop vectorize(assume_s
[PATCH] D49274: [CUDA] Provide integer SIMD functions for CUDA-9.2
tra created this revision. tra added reviewers: jlebar, bkramer. Herald added subscribers: bixia, sanjoy. CUDA-9.2 made all integer SIMD functions into compiler builtins, so clang no longer has access to the implementation of these functions in either headers of libdevice and has to provide its own implementation. This is mostly a 1:1 mapping to a corresponding PTX instructions with an exception of vhadd2/vhadd4 that don't have an equivalent instruction and had to be implemented with a bit hack. Performance of this implementation will be suboptimal for SM_50 and newer GPUs where PTXAS generates noticeably worse code for the SIMD instructions compared to the code it generates for the inline assembly generated by nvcc (or used to come with CUDA headers). https://reviews.llvm.org/D49274 Files: clang/lib/Headers/__clang_cuda_device_functions.h clang/lib/Headers/__clang_cuda_libdevice_declares.h Index: clang/lib/Headers/__clang_cuda_libdevice_declares.h === --- clang/lib/Headers/__clang_cuda_libdevice_declares.h +++ clang/lib/Headers/__clang_cuda_libdevice_declares.h @@ -372,6 +372,7 @@ __device__ unsigned int __nv_urhadd(unsigned int __a, unsigned int __b); __device__ unsigned int __nv_usad(unsigned int __a, unsigned int __b, unsigned int __c); +#if CUDA_VERSION >= 9000 && CUDA_VERSION < 9020 __device__ int __nv_vabs2(int __a); __device__ int __nv_vabs4(int __a); __device__ int __nv_vabsdiffs2(int __a, int __b); @@ -454,12 +455,12 @@ __device__ int __nv_vsubss4(int __a, int __b); __device__ int __nv_vsubus2(int __a, int __b); __device__ int __nv_vsubus4(int __a, int __b); +#endif // CUDA_VERSION __device__ double __nv_y0(double __a); __device__ float __nv_y0f(float __a); __device__ double __nv_y1(double __a); __device__ float __nv_y1f(float __a); __device__ float __nv_ynf(int __a, float __b); __device__ double __nv_yn(int __a, double __b); - } // extern "C" #endif // __CLANG_CUDA_LIBDEVICE_DECLARES_H__ Index: clang/lib/Headers/__clang_cuda_device_functions.h === --- clang/lib/Headers/__clang_cuda_device_functions.h +++ clang/lib/Headers/__clang_cuda_device_functions.h @@ -803,6 +803,8 @@ unsigned int __c) { return __nv_usad(__a, __b, __c); } + +#if CUDA_VERSION >= 9000 && CUDA_VERSION < 9020 __DEVICE__ unsigned int __vabs2(unsigned int __a) { return __nv_vabs2(__a); } __DEVICE__ unsigned int __vabs4(unsigned int __a) { return __nv_vabs4(__a); } __DEVICE__ unsigned int __vabsdiffs2(unsigned int __a, unsigned int __b) { @@ -1041,6 +1043,382 @@ __DEVICE__ unsigned int __vsubus4(unsigned int __a, unsigned int __b) { return __nv_vsubus4(__a, __b); } +#else // CUDA_VERSION >= 9020 +// CUDA no longer provides inline assembly (or bitcode) implementation of these +// functions, so we have to reimplment them. The implementation is naive and is +// not optimized for performance. + +// Helper function to convert N-bit boolean subfields into all-0 or all-1. +// E.g. __bool2mask(0x01000100,8) -> 0xff00ff00 +// __bool2mask(0x0001,16) -> 0x +__DEVICE__ unsigned int __bool2mask(unsigned int __a, int shift) { + return (__a << shift) - __a; +} +__DEVICE__ unsigned int __vabs2(unsigned int __a) { + unsigned int r; + asm("vabsdiff2.s32.s32.s32 %0,%1,0,0;" : "=r"(r) : "r"(__a)); + return r; +} +__DEVICE__ unsigned int __vabs4(unsigned int __a) { + unsigned int r; + asm("vabsdiff4.s32.s32.s32 %0,%1,0,0;" : "=r"(r) : "r"(__a)); + return r; +} +__DEVICE__ unsigned int __vabsdiffs2(unsigned int __a, unsigned int __b) { + unsigned int r; + asm("vabsdiff2.s32.s32.s32 %0,%1,%2,0;" : "=r"(r) : "r"(__a), "r"(__b)); + return r; +} + +__DEVICE__ unsigned int __vabsdiffs4(unsigned int __a, unsigned int __b) { + unsigned int r; + asm("vabsdiff4.s32.s32.s32 %0,%1,%2,0;" : "=r"(r) : "r"(__a), "r"(__b)); + return r; +} +__DEVICE__ unsigned int __vabsdiffu2(unsigned int __a, unsigned int __b) { + unsigned int r; + asm("vabsdiff2.u32.u32.u32.sat %0,%1,%2,0;" : "=r"(r) : "r"(__a), "r"(__b)); + return r; +} +__DEVICE__ unsigned int __vabsdiffu4(unsigned int __a, unsigned int __b) { + unsigned int r; + asm("vabsdiff4.u32.u32.u32.sat %0,%1,%2,0;" : "=r"(r) : "r"(__a), "r"(__b)); + return r; +} +__DEVICE__ unsigned int __vabsss2(unsigned int __a) { + unsigned int r; + asm("vabsdiff2.s32.s32.s32.sat %0,%1,0,0;" : "=r"(r) : "r"(__a)); + return r; +} +__DEVICE__ unsigned int __vabsss4(unsigned int __a) { + unsigned int r; + asm("vabsdiff2.s32.s32.s32.sat %0,%1,0,0;" : "=r"(r) : "r"(__a)); + return r; +} +__DEVICE__ unsigned int __vadd2(unsigned int __a, unsigned int __b) { + unsigned int r; + asm("vadd2.u32.u32.u32 %0,%1,%2,0;" : "=r"(r) : "r"(__a), "r"(__b)); + return r; +} +__DEVICE__ unsigned int __vadd4(unsigned int __a, unsigned int __b) { + unsigned int r; + asm("vadd4.u32.u
[PATCH] D49227: Override a bit fields layout from an external source
smeenai resigned from this revision. smeenai added a comment. I'm not familiar enough with this to review it. Repository: rC Clang https://reviews.llvm.org/D49227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336962 - PR38136: improve handling of template argument deduction of non-trailing
Author: rsmith Date: Thu Jul 12 16:32:39 2018 New Revision: 336962 URL: http://llvm.org/viewvc/llvm-project?rev=336962&view=rev Log: PR38136: improve handling of template argument deduction of non-trailing function parameter packs. This makes our handling of non-trailing function parameter packs consistent between the case of deduction at the top level in a function call and other cases where deduction encounters a non-trailing function parameter pack. Instead of treating a non-trailing pack and all later parameters as being non-deduced, we treat a non-trailing pack as exactly matching any explicitly-specified template arguments (or being an empty pack if there are no such arguments). This corresponds to the "never deduced" rule in [temp.deduct.call]p1, but generalized to all deduction contexts. Non-trailing template argument packs still result in the entire template argument list being treated as non-deduced, as specified in [temp.deduct.type]p9. Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=336962&r1=336961&r2=336962&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Thu Jul 12 16:32:39 2018 @@ -941,12 +941,6 @@ DeduceTemplateArguments(Sema &S, SmallVectorImpl &Deduced, unsigned TDF, bool PartialOrdering = false) { - // Fast-path check to see if we have too many/too few arguments. - if (NumParams != NumArgs && - !(NumParams && isa(Params[NumParams - 1])) && - !(NumArgs && isa(Args[NumArgs - 1]))) -return Sema::TDK_MiscellaneousDeductionFailure; - // C++0x [temp.deduct.type]p10: // Similarly, if P has a form that contains (T), then each parameter type // Pi of the respective parameter-type- list of P is compared with the @@ -983,13 +977,6 @@ DeduceTemplateArguments(Sema &S, continue; } -// C++0x [temp.deduct.type]p5: -// The non-deduced contexts are: -// - A function parameter pack that does not occur at the end of the -// parameter-declaration-clause. -if (ParamIdx + 1 < NumParams) - return Sema::TDK_Success; - // C++0x [temp.deduct.type]p10: // If the parameter-declaration corresponding to Pi is a function // parameter pack, then the type of its declarator- id is compared with @@ -1000,15 +987,41 @@ DeduceTemplateArguments(Sema &S, QualType Pattern = Expansion->getPattern(); PackDeductionScope PackScope(S, TemplateParams, Deduced, Info, Pattern); -for (; ArgIdx < NumArgs; ++ArgIdx) { - // Deduce template arguments from the pattern. - if (Sema::TemplateDeductionResult Result -= DeduceTemplateArgumentsByTypeMatch(S, TemplateParams, Pattern, - Args[ArgIdx], Info, Deduced, - TDF, PartialOrdering)) -return Result; +if (ParamIdx + 1 == NumParams) { + for (; ArgIdx < NumArgs; ++ArgIdx) { +// Deduce template arguments from the pattern. +if (Sema::TemplateDeductionResult Result + = DeduceTemplateArgumentsByTypeMatch(S, TemplateParams, Pattern, + Args[ArgIdx], Info, Deduced, + TDF, PartialOrdering)) + return Result; - PackScope.nextPackElement(); +PackScope.nextPackElement(); + } +} else { + // C++0x [temp.deduct.type]p5: + // The non-deduced contexts are: + // - A function parameter pack that does not occur at the end of the + // parameter-declaration-clause. + // + // FIXME: There is no wording to say what we should do in this case. We + // choose to resolve this by applying the same rule that is applied for a + // function call: that is, deduce all contained packs to their + // explicitly-specified values (or to <> if there is no such value). + // + // This is seemingly-arbitrarily different from the case of a template-id + // with a non-trailing pack-expansion in its arguments, which renders the + // entire template-argument-list a non-deduced context. + + // If the parameter type contains an explicitly-specified pack that we + // could not expand, skip the number of parameters notionally created + // by the expansion. + Optional NumExpansions = Expansion->getNumExpansions(); + if (NumExpansions && !PackScope.isPartiallyExpanded()) { +for (unsigned I = 0; I !
[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization
efriedma added inline comments. Comment at: lib/CodeGen/CGBuilder.h:260 +CharUnits::fromQuantity(Offset.getSExtValue(; + } + Not sure about the new helper. We already have CreateStructGEP and CreateConstArrayGEP which do approximately what you want. Comment at: lib/CodeGen/CGDecl.cpp:901 isa(Init)) { -Builder.CreateDefaultAlignedStore(Init, Loc, isVolatile); +Builder.CreateAlignedStore(Init, Loc.getPointer(), Loc.getAlignment(), + isVolatile); `Builder.CreateStore(Init, Loc, isVolatile);` Repository: rC Clang https://reviews.llvm.org/D49209 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49227: Override a bit fields layout from an external source
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: test/CodeGenCXX/override-bit-field-layout.cpp:5-12 +struct S { + short a : 3; + short b : 5; +}; + +void use_structs() { + S ss[sizeof(S)]; Another relevant test: ``` struct T { virtual void f(); short x : 3; }; ``` Because we don't do vfptr layout adjustments when using an external layout source, I think this would have put `x` at offset 0 instead of at offset 8 prior to your patch. Repository: rC Clang https://reviews.llvm.org/D49227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49275: Thread safety: Run tests with both lock and capability attributes
aaronpuchert created this revision. aaronpuchert added reviewers: delesley, aaron.ballman. Herald added a subscriber: cfe-commits. Running all tests with both sets of attributes should make sure there is no regression in either variant. Repository: rC Clang https://reviews.llvm.org/D49275 Files: test/SemaCXX/warn-thread-safety-analysis.cpp Index: test/SemaCXX/warn-thread-safety-analysis.cpp === --- test/SemaCXX/warn-thread-safety-analysis.cpp +++ test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1,55 +1,58 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 %s -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 -DUSE_TRY_ACQUIRE_CAPABILITY %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s -#define LOCKABLE __attribute__((lockable)) #define SCOPED_LOCKABLE __attribute__((scoped_lockable)) #define GUARDED_BY(x)__attribute__((guarded_by(x))) #define GUARDED_VAR __attribute__((guarded_var)) #define PT_GUARDED_BY(x) __attribute__((pt_guarded_by(x))) #define PT_GUARDED_VAR __attribute__((pt_guarded_var)) #define ACQUIRED_AFTER(...) __attribute__((acquired_after(__VA_ARGS__))) #define ACQUIRED_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__))) -#define EXCLUSIVE_LOCK_FUNCTION(...)__attribute__((exclusive_lock_function(__VA_ARGS__))) -#define SHARED_LOCK_FUNCTION(...) __attribute__((shared_lock_function(__VA_ARGS__))) -#if USE_ASSERT_CAPABILITY +#if USE_CAPABILITY +#define LOCKABLE__attribute__((capability("mutex"))) #define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_capability(__VA_ARGS__))) #define ASSERT_SHARED_LOCK(...) __attribute__((assert_shared_capability(__VA_ARGS__))) -#else -#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_exclusive_lock(__VA_ARGS__))) -#define ASSERT_SHARED_LOCK(...) __attribute__((assert_shared_lock(__VA_ARGS__))) -#endif - -#ifdef USE_TRY_ACQUIRE_CAPABILITY +#define EXCLUSIVE_LOCK_FUNCTION(...)__attribute__((acquire_capability(__VA_ARGS__))) +#define SHARED_LOCK_FUNCTION(...) __attribute__((acquire_shared_capability(__VA_ARGS__))) #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((try_acquire_capability(__VA_ARGS__))) #define SHARED_TRYLOCK_FUNCTION(...)__attribute__((try_acquire_shared_capability(__VA_ARGS__))) +#define EXCLUSIVE_UNLOCK_FUNCTION(...) __attribute__((release_capability(__VA_ARGS__))) +#define SHARED_UNLOCK_FUNCTION(...) __attribute__((release_shared_capability(__VA_ARGS__))) +#define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((requires_capability(__VA_ARGS__))) +#define SHARED_LOCKS_REQUIRED(...) __attribute__((requires_shared_capability(__VA_ARGS__))) #else +#define LOCKABLE__attribute__((lockable)) +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_exclusive_lock(__VA_ARGS__))) +#define ASSERT_SHARED_LOCK(...) __attribute__((assert_shared_lock(__VA_ARGS__))) +#define EXCLUSIVE_LOCK_FUNCTION(...)__attribute__((exclusive_lock_function(__VA_ARGS__))) +#define SHARED_LOCK_FUNCTION(...) __attribute__((shared_lock_function(__VA_ARGS__))) #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((exclusive_trylock_function(__VA_ARGS__))) #define SHARED_TRYLOCK_FUNCTION(...)__attribute__((shared_trylock_function(__VA_ARGS__))) +#define EXCLUSIVE_UNLOCK_FUNCTION(...) __attribute__((unlock_function(__VA_ARGS__))) +#define SHARED_UNLOCK_FUNCTION(...) __attribute__((unlock_function(__VA_ARGS__))) +#define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_required(__VA_ARG
[PATCH] D49227: Override a bit fields layout from an external source
aleksandr.urakov updated this revision to Diff 155324. aleksandr.urakov added a comment. Thank you! Yes, I have added a test case for that. If it's OK, can you commit this for me, please? I have no commit access. https://reviews.llvm.org/D49227 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/Inputs/override-bit-field-layout.layout test/CodeGenCXX/override-bit-field-layout.cpp Index: test/CodeGenCXX/override-bit-field-layout.cpp === --- /dev/null +++ test/CodeGenCXX/override-bit-field-layout.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s + +// CHECK: Type: struct S1 +// CHECK: FieldOffsets: [0, 11] +struct S1 { + short a : 3; + short b : 5; +}; + +// CHECK: Type: struct S2 +// CHECK: FieldOffsets: [64] +struct S2 { + virtual ~S2() = default; + short a : 3; +}; + +void use_structs() { + S1 s1s[sizeof(S1)]; + S2 s2s[sizeof(S2)]; +} Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout === --- /dev/null +++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout @@ -0,0 +1,16 @@ + +*** Dumping AST Record Layout +Type: struct S1 + +Layout: + +*** Dumping AST Record Layout +Type: struct S2 + +Layout: Index: lib/AST/RecordLayoutBuilder.cpp === --- lib/AST/RecordLayoutBuilder.cpp +++ lib/AST/RecordLayoutBuilder.cpp @@ -2677,7 +2677,7 @@ // Check to see if this bitfield fits into an existing allocation. Note: // MSVC refuses to pack bitfields of formal types with different sizes // into the same allocation. - if (!IsUnion && LastFieldIsNonZeroWidthBitfield && + if (!UseExternalLayout && !IsUnion && LastFieldIsNonZeroWidthBitfield && CurrentBitfieldSize == Info.Size && Width <= RemainingBitsInField) { placeFieldAtBitOffset(Context.toBits(Size) - RemainingBitsInField); RemainingBitsInField -= Width; @@ -2689,6 +2689,14 @@ placeFieldAtOffset(CharUnits::Zero()); Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. + } else if (UseExternalLayout) { +auto FieldBitOffset = External.getExternalFieldOffset(FD); +placeFieldAtBitOffset(FieldBitOffset); +auto NewSize = Context.toCharUnitsFromBits( +llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth())); +assert(NewSize >= Size && "bit field offset already allocated"); +Size = NewSize; +Alignment = std::max(Alignment, Info.Alignment); } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); Index: test/CodeGenCXX/override-bit-field-layout.cpp === --- /dev/null +++ test/CodeGenCXX/override-bit-field-layout.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s + +// CHECK: Type: struct S1 +// CHECK: FieldOffsets: [0, 11] +struct S1 { + short a : 3; + short b : 5; +}; + +// CHECK: Type: struct S2 +// CHECK: FieldOffsets: [64] +struct S2 { + virtual ~S2() = default; + short a : 3; +}; + +void use_structs() { + S1 s1s[sizeof(S1)]; + S2 s2s[sizeof(S2)]; +} Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout === --- /dev/null +++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout @@ -0,0 +1,16 @@ + +*** Dumping AST Record Layout +Type: struct S1 + +Layout: + +*** Dumping AST Record Layout +Type: struct S2 + +Layout: Index: lib/AST/RecordLayoutBuilder.cpp === --- lib/AST/RecordLayoutBuilder.cpp +++ lib/AST/RecordLayoutBuilder.cpp @@ -2677,7 +2677,7 @@ // Check to see if this bitfield fits into an existing allocation. Note: // MSVC refuses to pack bitfields of formal types with different sizes // into the same allocation. - if (!IsUnion && LastFieldIsNonZeroWidthBitfield && + if (!UseExternalLayout && !IsUnion && LastFieldIsNonZeroWidthBitfield && CurrentBitfieldSize == Info.Size && Width <= RemainingBitsInField) { placeFieldAtBitOffset(Context.toBits(Size) - RemainingBitsInField); RemainingBitsInField -= Width; @@ -2689,6 +2689,14 @@ placeFieldAtOffset(CharUnits::Zero()); Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. + } else if (UseExternalLayout) { +auto FieldBitOffset = External.getExternalFieldOffset(FD); +placeFieldAtBitOffset(FieldBitOffset); +auto NewSize = Context.toCharUnitsFromBits( +llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth())); +assert(NewSize >= S