r293995 - clang-format: [Proto] Also supports implicit string literal concatenation
Author: djasper Date: Fri Feb 3 02:29:02 2017 New Revision: 293995 URL: http://llvm.org/viewvc/llvm-project?rev=293995&view=rev Log: clang-format: [Proto] Also supports implicit string literal concatenation Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestProto.cpp Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=293995&r1=293994&r2=293995&view=diff == --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Feb 3 02:29:02 2017 @@ -2391,7 +2391,8 @@ bool TokenAnnotator::mustBreakBefore(con Right.Next->is(tok::string_literal)) return true; } else if (Style.Language == FormatStyle::LK_Cpp || - Style.Language == FormatStyle::LK_ObjC) { + Style.Language == FormatStyle::LK_ObjC || + Style.Language == FormatStyle::LK_Proto) { if (Left.isStringLiteral() && (Right.isStringLiteral() || Right.is(TT_ObjCStringLiteral))) return true; Modified: cfe/trunk/unittests/Format/FormatTestProto.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=293995&r1=293994&r2=293995&view=diff == --- cfe/trunk/unittests/Format/FormatTestProto.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestProto.cpp Fri Feb 3 02:29:02 2017 @@ -135,6 +135,9 @@ TEST_F(FormatTestProto, MessageFieldAttr "key: 'a' //\n" " }\n" "];"); + verifyFormat("optional string test = 1 [default =\n" + " \"test\"\n" + " \"test\"];"); } TEST_F(FormatTestProto, DoesntWrapFileOptions) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29486: [clang-format] Re-align broken comment lines where appropriate.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Thanks and sorry if I caused this :( https://reviews.llvm.org/D29486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29486: [clang-format] Re-align broken comment lines where appropriate.
krasimir added a comment. It's very unlikely that you caused this. After re-flowing lots of cases started going over alternative code paths, and this looks like an instance of that. https://reviews.llvm.org/D29486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r293997 - [clang-format] Re-align broken comment lines where appropriate.
Author: krasimir Date: Fri Feb 3 04:18:25 2017 New Revision: 293997 URL: http://llvm.org/viewvc/llvm-project?rev=293997&view=rev Log: [clang-format] Re-align broken comment lines where appropriate. Summary: The comment aligner was skipping over newly broken comment lines. This patch fixes that. source: ``` int ab; // line int a; // long long ``` format with column limit 15 before: ``` int ab; // line int a; // long // long ``` format with column limit 15 after: ``` int ab; // line int a; // long // long ``` Reviewers: djasper Reviewed By: djasper Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D29486 Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=293997&r1=293996&r2=293997&view=diff == --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original) +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Fri Feb 3 04:18:25 2017 @@ -164,8 +164,9 @@ void WhitespaceManager::calculateLineBre const WhitespaceManager::Change *LastBlockComment = nullptr; for (auto &Change : Changes) { // Reset the IsTrailingComment flag for changes inside of trailing comments -// so they don't get realigned later. -if (Change.IsInsideToken) +// so they don't get realigned later. Comment line breaks however still need +// to be aligned. +if (Change.IsInsideToken && Change.NewlinesBefore == 0) Change.IsTrailingComment = false; Change.StartOfBlockComment = nullptr; Change.IndentationOffset = 0; Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293997&r1=293996&r2=293997&view=diff == --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Feb 3 04:18:25 2017 @@ -11852,6 +11852,40 @@ TEST_F(FormatTest, AlignTrailingComments " // line 3\n" " b);", getLLVMStyleWithColumns(40))); + + // Align newly broken trailing comments. + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n", +format("int ab; // line\n" + "int a; // long long\n", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"// long", +format("int ab; // line\n" + "int a; // long long\n" + " // long", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"pt c; // long", +format("int ab; // line\n" + "int a; // long long\n" + "pt c; // long", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"\n" +"// long", +format("int ab; // line\n" + "int a; // long long\n" + "\n" + "// long", + getLLVMStyleWithColumns(15))); } } // end namespace } // end namespace format ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29486: [clang-format] Re-align broken comment lines where appropriate.
This revision was automatically updated to reflect the committed changes. Closed by commit rL293997: [clang-format] Re-align broken comment lines where appropriate. (authored by krasimir). Changed prior to commit: https://reviews.llvm.org/D29486?vs=86942&id=86945#toc Repository: rL LLVM https://reviews.llvm.org/D29486 Files: cfe/trunk/lib/Format/WhitespaceManager.cpp cfe/trunk/unittests/Format/FormatTest.cpp Index: cfe/trunk/lib/Format/WhitespaceManager.cpp === --- cfe/trunk/lib/Format/WhitespaceManager.cpp +++ cfe/trunk/lib/Format/WhitespaceManager.cpp @@ -164,8 +164,9 @@ const WhitespaceManager::Change *LastBlockComment = nullptr; for (auto &Change : Changes) { // Reset the IsTrailingComment flag for changes inside of trailing comments -// so they don't get realigned later. -if (Change.IsInsideToken) +// so they don't get realigned later. Comment line breaks however still need +// to be aligned. +if (Change.IsInsideToken && Change.NewlinesBefore == 0) Change.IsTrailingComment = false; Change.StartOfBlockComment = nullptr; Change.IndentationOffset = 0; Index: cfe/trunk/unittests/Format/FormatTest.cpp === --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -11852,6 +11852,40 @@ " // line 3\n" " b);", getLLVMStyleWithColumns(40))); + + // Align newly broken trailing comments. + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n", +format("int ab; // line\n" + "int a; // long long\n", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"// long", +format("int ab; // line\n" + "int a; // long long\n" + " // long", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"pt c; // long", +format("int ab; // line\n" + "int a; // long long\n" + "pt c; // long", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"\n" +"// long", +format("int ab; // line\n" + "int a; // long long\n" + "\n" + "// long", + getLLVMStyleWithColumns(15))); } } // end namespace } // end namespace format Index: cfe/trunk/lib/Format/WhitespaceManager.cpp === --- cfe/trunk/lib/Format/WhitespaceManager.cpp +++ cfe/trunk/lib/Format/WhitespaceManager.cpp @@ -164,8 +164,9 @@ const WhitespaceManager::Change *LastBlockComment = nullptr; for (auto &Change : Changes) { // Reset the IsTrailingComment flag for changes inside of trailing comments -// so they don't get realigned later. -if (Change.IsInsideToken) +// so they don't get realigned later. Comment line breaks however still need +// to be aligned. +if (Change.IsInsideToken && Change.NewlinesBefore == 0) Change.IsTrailingComment = false; Change.StartOfBlockComment = nullptr; Change.IndentationOffset = 0; Index: cfe/trunk/unittests/Format/FormatTest.cpp === --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -11852,6 +11852,40 @@ " // line 3\n" " b);", getLLVMStyleWithColumns(40))); + + // Align newly broken trailing comments. + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n", +format("int ab; // line\n" + "int a; // long long\n", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"// long", +format("int ab; // line\n" + "int a; // long long\n" + " // long", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"pt c; // long", +format("int ab; // line\n" + "int a; // long long\n" + "pt c; // long", + getLLVMStyleWithColumns(15))); + EXPECT_EQ("int ab; // line\n" +"int a; // long\n" +"// long\n" +"\n" +"
[PATCH] D29451: Add a prototype for clangd v0.1
djasper accepted this revision. djasper added a comment. Just a few nits. I think this looks like a great starting point! Comment at: clangd/ClangDMain.cpp:33 + llvm::make_unique(Outs, Logs, Store)); + // FIXME: textDocument/didClose + Dispatcher.registerHandler( Maybe add "Implement" :) Comment at: clangd/ClangDMain.cpp:52 +llvm::StringRef LineRef(Line); +if (LineRef.trim().empty()) + continue; Nit: string also has "empty()", right? Maybe only convert to StringRef after this check. Comment at: clangd/ClangDMain.cpp:62 + +std::cin.ignore(2); // Skip \r\n + Should you somehow actually read them and assert that they are "\r\n"? Comment at: clangd/ClangDMain.cpp:74 + // Log the message. + Logs << "<-- "; + Logs.write(JSON.data(), JSON.size()); So you currently always print this to std::err.. Should you guard this in DEBUG or something? Comment at: clangd/DocumentStore.h:25 + /// Add a document to the store. Overwrites existing contents. + void setDocument(StringRef Uri, StringRef Text) { Docs[Uri] = Text; } + /// Delete a document from the store. I'd call this addDocument for symmetry with removeDocument. The fact that you phrase the comment that way, also make me think that this will be the more intuitive name. Comment at: clangd/JSONRPCDispatcher.cpp:89 + Id = dyn_cast(Value); +} else if (KeyValue == "params") { + if (!Method) I'd pull out this case and move it to the front to show that this is a guaranteed early exit and the function can never fall through to the following code. But I am not sure. Comment at: clangd/ProtocolHandlers.h:27 + +struct InitializeHandler : Handler { + InitializeHandler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs) In the long run, we probably don't want to specify all of these in the same file, but for now this seems fine. Comment at: test/clangd/formatting.txt:9 +# CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{ +# CHECK: "textDocumentSync": 1, +# CHECK: "documentFormattingProvider": true, How did you come up with this indentation? https://reviews.llvm.org/D29451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29451: Add a prototype for clangd v0.1
Adi added a comment. I had some spare time ... I hope you don't mind the comments. Hopefully you will find something useful :) Comment at: clangd/ClangDMain.cpp:65-70 +std::vector JSON; +JSON.resize(Len); +std::cin.read(JSON.data(), Len); + +// Insert a trailing null byte. YAML parser requires this. +JSON.push_back('\0'); Avoid unnecessary JSON.resize(Len) & potential reallocation during JSON.push_back('\0') by allocating enough space in advance: std::vector JSON(Len + 1); Comment at: clangd/ClangDMain.cpp:73-77 + // Log the message. + Logs << "<-- "; + Logs.write(JSON.data(), JSON.size()); + Logs << '\n'; + Logs.flush(); Since llvm::StringRef does not own the data, I think it would make more sense to do the logging after Dispatcher.call(). Comment at: clangd/ClangDMain.cpp:80 + // Finally, execute the action for this JSON message. + Dispatcher.call(llvm::StringRef(JSON.data(), JSON.size() - 1)); +} You are building llvm::StringRef without the trailing null byte here, yet comment above states that YAML parser requires a null byte. Comment at: clangd/ClangDMain.cpp:80 + // Finally, execute the action for this JSON message. + Dispatcher.call(llvm::StringRef(JSON.data(), JSON.size() - 1)); +} Adi wrote: > You are building llvm::StringRef without the trailing null byte here, yet > comment above states that YAML parser requires a null byte. Perhaps make a log entry if Dispatcher.call() failed. Comment at: clangd/JSONRPCDispatcher.cpp:32-40 + Logs << "Method ignored.\n"; + // Return that this method is unsupported. + writeMessage( + R"({"jsonrpc":"2.0","id":)" + ID + + R"(,"error":{"code":-32601}})"); +} + I would extract this implementation to the separate handler class (e.g. OperationNotSupportedHandler) and make this an interface class. It would make code and intent more explicit. Comment at: clangd/JSONRPCDispatcher.cpp:82-83 + // This should be "2.0". Always. + Version = dyn_cast(Value); + if (Version->getRawValue() != "2.0") +return false; dyn_cast might fail and leave you with Version set to nullptr. Using static_cast is better approach if you are sure the cast will be successful. Comment at: clangd/JSONRPCDispatcher.cpp:112-120 + if (!Method) +return false; + llvm::SmallString<10> MethodStorage; + auto I = Handlers.find(Method->getValue(MethodStorage)); + auto &Handler = I != Handlers.end() ? I->second : UnknownHandler; + if (Id) +Handler->handleMethod(nullptr, Id->getRawValue()); Perhaps refactor this code and the one above into the separate method to gain some more readability. Comment at: clangd/JSONRPCDispatcher.h:25-32 + virtual ~Handler() = default; + + /// Called when the server receives a method call. This is supposed to return + /// a result on Outs. + virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID); + /// Called when the server receives a notification. No result should be + /// written to Outs. To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g. ``` template class Handler { public: Handler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs) : Outs(Outs), Logs(Logs) {} virtual ~Handler() = default; void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) { static_cast(this)->handleMethod(Params, ID); } void handleNotification(const llvm::yaml::MappingMode *Params) { static_cast(this)->handleNotification(Params); } protected: llvm::raw_ostream &Outs; llvm::raw_ostream &Logs; void writeMessage(const Twine &Message); }; ``` And then use it as: ``` struct MyConcreteHandler : public Handler { MyConcreteHandler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs /* other params if necessary */) : Handler(Outs, Logs) /* init other members */ {} void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) { // impl } void handleNotification(const llvm::yaml::MappingMode *Params) { // impl } }; ``` Comment at: clangd/JSONRPCDispatcher.h:29 + /// a result on Outs. + virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID); + /// Called when the server receives a notification. No result should be const ptr/ref to Params? Comment at: clangd/JSONRPCDispatcher.h:32 + /// written to Outs. + virtual void handleNotification(llvm::yaml::MappingNode *Params); + const ptr/ref to Params? Comment at: clangd/Protocol.cpp:11 +// This file contains the serialization code for the LSP structs. +// FIXME
[PATCH] D29451: Add a prototype for clangd v0.1
klimek accepted this revision. klimek added a comment. LG. Couple of questions. Comment at: clangd/ClangDMain.cpp:65 +// Now read the JSON. +std::vector JSON; +JSON.resize(Len); Adi wrote: > Avoid unnecessary JSON.resize(Len) & potential reallocation during > JSON.push_back('\0') by allocating enough space in advance: std::vector > JSON(Len + 1); Shouldn't that be unsigned char for raw bytes? Comment at: clangd/JSONRPCDispatcher.h:29 + /// a result on Outs. + virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID); + /// Called when the server receives a notification. No result should be klimek wrote: > Adi wrote: > > const ptr/ref to Params? > Can we make those Params pointers-to-const? Here and below, document what the default implementations do. Comment at: clangd/JSONRPCDispatcher.h:29-32 + virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID); + /// Called when the server receives a notification. No result should be + /// written to Outs. + virtual void handleNotification(llvm::yaml::MappingNode *Params); Adi wrote: > const ptr/ref to Params? Can we make those Params pointers-to-const? https://reviews.llvm.org/D29451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29451: Add a prototype for clangd v0.1
klimek added inline comments. Comment at: clangd/JSONRPCDispatcher.h:25-32 + virtual ~Handler() = default; + + /// Called when the server receives a method call. This is supposed to return + /// a result on Outs. + virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID); + /// Called when the server receives a notification. No result should be + /// written to Outs. Adi wrote: > To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g. > > ``` > template > class Handler { > public: > Handler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs) > : Outs(Outs), Logs(Logs) {} > virtual ~Handler() = default; > > void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) { > static_cast(this)->handleMethod(Params, ID); > } > void handleNotification(const llvm::yaml::MappingMode *Params) { > static_cast(this)->handleNotification(Params); > } > > protected: > llvm::raw_ostream &Outs; > llvm::raw_ostream &Logs; > > void writeMessage(const Twine &Message); > }; > ``` > And then use it as: > > ``` > struct MyConcreteHandler : public Handler { > MyConcreteHandler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs /* > other params if necessary */) > : Handler(Outs, Logs) /* init other members */ > {} > > void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) { > // impl > } > void handleNotification(const llvm::yaml::MappingMode *Params) { > // impl > } > }; > ``` > To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g. Why would virtual dispatch be a problem here? It seems strictly simpler this way. https://reviews.llvm.org/D29451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32
yaron.keren added a comment. This code is actually used with Windows as well as Linux (with the exception of line 218), see the comment blocks above for detailed include dirs from all platforms from which it was derived. Please make sure include dirs between gcc and clang match after the patch: echo | gcc -E -x c -Wp,-v - echo | clang -E -x c -Wp,-v - and echo | gcc -E -x c++ -Wp,-v - echo | clang -E -x c++ -Wp,-v - https://reviews.llvm.org/D29464 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29451: Add a prototype for clangd v0.1
Adi added inline comments. Comment at: clangd/JSONRPCDispatcher.h:25-32 + virtual ~Handler() = default; + + /// Called when the server receives a method call. This is supposed to return + /// a result on Outs. + virtual void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID); + /// Called when the server receives a notification. No result should be + /// written to Outs. klimek wrote: > Adi wrote: > > To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g. > > > > ``` > > template > > class Handler { > > public: > > Handler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs) > > : Outs(Outs), Logs(Logs) {} > > virtual ~Handler() = default; > > > > void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) > > { > > static_cast(this)->handleMethod(Params, ID); > > } > > void handleNotification(const llvm::yaml::MappingMode *Params) { > > static_cast(this)->handleNotification(Params); > > } > > > > protected: > > llvm::raw_ostream &Outs; > > llvm::raw_ostream &Logs; > > > > void writeMessage(const Twine &Message); > > }; > > ``` > > And then use it as: > > > > ``` > > struct MyConcreteHandler : public Handler { > > MyConcreteHandler(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs /* > > other params if necessary */) > > : Handler(Outs, Logs) /* init other members */ > > {} > > > > void handleMethod(const llvm::yaml::MappingMode *Params, StringRef ID) > > { > > // impl > > } > > void handleNotification(const llvm::yaml::MappingMode *Params) { > > // impl > > } > > }; > > ``` > > To avoid virtual dispatch one may rewrite this class in terms of CRTP. E.g. > > Why would virtual dispatch be a problem here? It seems strictly simpler this > way. > I've mentioned it only as an alternative approach. I was not implying that virtual dispatch is a problem. It's a matter of taste in this case really. Comment at: clangd/Protocol.cpp:11 +// This file contains the serialization code for the LSP structs. +// FIXME: This is extremely repetetive and ugly. Is there a better way? +// Adi wrote: > To some degree it could be refactored. I would go for CRTP again here. It > would simplify the code a little bit. E.g. > > ``` > template > struct Params { > static llvm::Optional parse(const llvm::yaml::MappingNode& node) { > T Result; > for (auto& NextKeyValue : node) { > auto *KeyString = > dyn_cast(NextKeyValue.getKey()); > if (!KeyString) > return llvm::None; > > llvm::SmallString<10> KeyStorage; > StringRef KeyValue = KeyString->getValue(KeyStorage); > auto *Value = > dyn_cast_or_null(NextKeyValue.getValue()); > if (!Value) > return llvm::None; > > Result = static_cast(this)->setParam(KeyValue, *Value); // or > make Result an argument if worried about RVO > if (!Result) // we can make this more neat by placing it inside a > for loop condition (ranged-for doesn't allow this) > return llvm::none; > } > return Result; > } > }; > > struct TextDocumentIdentifier : public Params { > /// The text document's URI. > std::string uri; > > llvm::Optional setParam(StringRef KeyValue, const > llvm::yaml::ScalarNode& Value) { > TextDocumentIdentifier Result; > llvm::SmallString<10> Storage; > if (KeyValue == "uri") { > Result.uri = Value.getValue(Storage); > } else if (KeyValue == "version") { > // FIXME: parse version, but only for > VersionedTextDocumentIdentifiers. > } else { > return llvm::None; > } > return Result; > } > }; > > struct Position : public Params { > /// Line position in a document (zero-based). > int line; > > /// Character offset on a line in a document (zero-based). > int character; > > llvm::Optional setParam(StringRef KeyValue, const > llvm::yaml::ScalarNode& Value) { > Position Result; > llvm::SmallString<10> Storage; > if (KeyValue == "line") { > long long Val; > if (llvm::getAsSignedInteger(Value.getValue(Storage), 0, Val)) > return llvm::None; > Result.line = Val; > } else if (KeyValue == "character") { > long long Val; > if (llvm::getAsSignedInteger(Value.getValue(Storage), 0, Val)) > return llvm::None; > Result.character = Val; > } else { > return llvm::None; > } > return Result; > } > }; > ``` > > I am not really familiar with all of the dependencies here but I may suggest > that one might also try to generalize this approach even further by > encapsulating the
[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. A couple of nits. Please address Aaron's comment as well. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:53 + for (const auto &NoExceptRange : NoExceptRanges) { +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) nit: remove the FIXME Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<< FixItHint::CreateRemoval(NoExceptRange); nit: `nothrow` (without a dash), no colon needed (it will look weird, since the location is mentioned _before_ the message, not after it) https://reviews.llvm.org/D19201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32
mati865 added a comment. Adding // c:\mingw32\lib\gcc\i686-w64-mingw32\4.9.1\include // c:\mingw32\lib\gcc\i686-w64-mingw32\4.9.1\include-fixed for Clang is wrong on Windows because including limits.h will not show an error and won't really include mingw-w64 limits.h Output of code from my earlier comment without patch: error: use of undeclared identifier 'PATH_MAX' int main() { char buf[PATH_MAX]; } I haven't used cross compiled Clang on Linux so I added #ifndef LLVM_ON_WIN32 guard for this code. With this guard Clang on Windows uses include paths like native Clang on Linux Include directories on public paste: https://reviews.llvm.org/P7968 https://reviews.llvm.org/D29464 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r294008 - [Sema][ObjC++] Typo correction should handle ivars and properties
Hi Hans, Is there any chance we can merge this for 4.0? It fixed a nasty bug where clang didn't catch invalid ObjC++ code during semantic analysis which led to invalid object files or crashes in CodeGen. Cheers, Alex On 3 February 2017 at 14:22, Alex Lorenz via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: arphaman > Date: Fri Feb 3 08:22:33 2017 > New Revision: 294008 > > URL: http://llvm.org/viewvc/llvm-project?rev=294008&view=rev > Log: > [Sema][ObjC++] Typo correction should handle ivars and properties > > After r260016 and r260017 disabled typo correction for ivars and properties > clang didn't report errors about unresolved identifier in the base of ivar > and > property ref expressions. This meant that clang invoked CodeGen on invalid > AST > which then caused a crash. > > This commit re-enables typo correction for ivars and properites, and fixes > the > PR25113 & PR26486 (that were originally fixed in r260017 and r260016) in a > different manner by transforming the Objective-C ivar reference expression > with > 'IsFreeIvar' preserved. > > rdar://30310772 > > Modified: > cfe/trunk/lib/Sema/SemaExprCXX.cpp > cfe/trunk/lib/Sema/TreeTransform.h > cfe/trunk/test/SemaObjCXX/typo-correction.mm > > Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaExprCXX.cpp?rev=294008&r1=294007&r2=294008&view=diff > > == > --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb 3 08:22:33 2017 > @@ -7250,14 +7250,6 @@ public: > >ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); } > > - ExprResult TransformObjCPropertyRefExpr(ObjCPropertyRefExpr *E) { > -return Owned(E); > - } > - > - ExprResult TransformObjCIvarRefExpr(ObjCIvarRefExpr *E) { > -return Owned(E); > - } > - >ExprResult Transform(Expr *E) { > ExprResult Res; > while (true) { > > Modified: cfe/trunk/lib/Sema/TreeTransform.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > TreeTransform.h?rev=294008&r1=294007&r2=294008&view=diff > > == > --- cfe/trunk/lib/Sema/TreeTransform.h (original) > +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Feb 3 08:22:33 2017 > @@ -2982,16 +2982,17 @@ public: >ExprResult RebuildObjCIvarRefExpr(Expr *BaseArg, ObjCIvarDecl *Ivar, >SourceLocation IvarLoc, >bool IsArrow, bool IsFreeIvar) { > -// FIXME: We lose track of the IsFreeIvar bit. > CXXScopeSpec SS; > DeclarationNameInfo NameInfo(Ivar->getDeclName(), IvarLoc); > -return getSema().BuildMemberReferenceExpr(BaseArg, > BaseArg->getType(), > - /*FIXME:*/IvarLoc, IsArrow, > - SS, SourceLocation(), > - /*FirstQualifierInScope=*/ > nullptr, > - NameInfo, > - /*TemplateArgs=*/nullptr, > - /*S=*/nullptr); > +ExprResult Result = getSema().BuildMemberReferenceExpr( > +BaseArg, BaseArg->getType(), > +/*FIXME:*/ IvarLoc, IsArrow, SS, SourceLocation(), > +/*FirstQualifierInScope=*/nullptr, NameInfo, > +/*TemplateArgs=*/nullptr, > +/*S=*/nullptr); > +if (IsFreeIvar && Result.isUsable()) > + cast(Result.get())->setIsFreeIvar(IsFreeIvar); > +return Result; >} > >/// \brief Build a new Objective-C property reference expression. > > Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > SemaObjCXX/typo-correction.mm?rev=294008&r1=294007&r2=294008&view=diff > > == > --- cfe/trunk/test/SemaObjCXX/typo-correction.mm (original) > +++ cfe/trunk/test/SemaObjCXX/typo-correction.mm Fri Feb 3 08:22:33 2017 > @@ -21,3 +21,18 @@ public: >self.m_prop2 = new ClassB(m_prop1); // expected-error {{use of > undeclared identifier 'm_prop1'; did you mean '_m_prop1'?}} > } > @end > + > +// rdar://30310772 > + > +@interface InvalidNameInIvarAndPropertyBase > +{ > +@public > + float _a; > +} > +@property float _b; > +@end > + > +void invalidNameInIvarAndPropertyBase() { > + float a = ((InvalidNameInIvarAndPropertyBase*)node)->_a; // > expected-error {{use of undeclared identifier 'node'}} > + float b = ((InvalidNameInIvarAndPropertyBase*)node)._b; // > expected-error {{use of undeclared identifier 'node'}} > +} > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___
r294008 - [Sema][ObjC++] Typo correction should handle ivars and properties
Author: arphaman Date: Fri Feb 3 08:22:33 2017 New Revision: 294008 URL: http://llvm.org/viewvc/llvm-project?rev=294008&view=rev Log: [Sema][ObjC++] Typo correction should handle ivars and properties After r260016 and r260017 disabled typo correction for ivars and properties clang didn't report errors about unresolved identifier in the base of ivar and property ref expressions. This meant that clang invoked CodeGen on invalid AST which then caused a crash. This commit re-enables typo correction for ivars and properites, and fixes the PR25113 & PR26486 (that were originally fixed in r260017 and r260016) in a different manner by transforming the Objective-C ivar reference expression with 'IsFreeIvar' preserved. rdar://30310772 Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/lib/Sema/TreeTransform.h cfe/trunk/test/SemaObjCXX/typo-correction.mm Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=294008&r1=294007&r2=294008&view=diff == --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb 3 08:22:33 2017 @@ -7250,14 +7250,6 @@ public: ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); } - ExprResult TransformObjCPropertyRefExpr(ObjCPropertyRefExpr *E) { -return Owned(E); - } - - ExprResult TransformObjCIvarRefExpr(ObjCIvarRefExpr *E) { -return Owned(E); - } - ExprResult Transform(Expr *E) { ExprResult Res; while (true) { Modified: cfe/trunk/lib/Sema/TreeTransform.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=294008&r1=294007&r2=294008&view=diff == --- cfe/trunk/lib/Sema/TreeTransform.h (original) +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Feb 3 08:22:33 2017 @@ -2982,16 +2982,17 @@ public: ExprResult RebuildObjCIvarRefExpr(Expr *BaseArg, ObjCIvarDecl *Ivar, SourceLocation IvarLoc, bool IsArrow, bool IsFreeIvar) { -// FIXME: We lose track of the IsFreeIvar bit. CXXScopeSpec SS; DeclarationNameInfo NameInfo(Ivar->getDeclName(), IvarLoc); -return getSema().BuildMemberReferenceExpr(BaseArg, BaseArg->getType(), - /*FIXME:*/IvarLoc, IsArrow, - SS, SourceLocation(), - /*FirstQualifierInScope=*/nullptr, - NameInfo, - /*TemplateArgs=*/nullptr, - /*S=*/nullptr); +ExprResult Result = getSema().BuildMemberReferenceExpr( +BaseArg, BaseArg->getType(), +/*FIXME:*/ IvarLoc, IsArrow, SS, SourceLocation(), +/*FirstQualifierInScope=*/nullptr, NameInfo, +/*TemplateArgs=*/nullptr, +/*S=*/nullptr); +if (IsFreeIvar && Result.isUsable()) + cast(Result.get())->setIsFreeIvar(IsFreeIvar); +return Result; } /// \brief Build a new Objective-C property reference expression. Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/typo-correction.mm?rev=294008&r1=294007&r2=294008&view=diff == --- cfe/trunk/test/SemaObjCXX/typo-correction.mm (original) +++ cfe/trunk/test/SemaObjCXX/typo-correction.mm Fri Feb 3 08:22:33 2017 @@ -21,3 +21,18 @@ public: self.m_prop2 = new ClassB(m_prop1); // expected-error {{use of undeclared identifier 'm_prop1'; did you mean '_m_prop1'?}} } @end + +// rdar://30310772 + +@interface InvalidNameInIvarAndPropertyBase +{ +@public + float _a; +} +@property float _b; +@end + +void invalidNameInIvarAndPropertyBase() { + float a = ((InvalidNameInIvarAndPropertyBase*)node)->_a; // expected-error {{use of undeclared identifier 'node'}} + float b = ((InvalidNameInIvarAndPropertyBase*)node)._b; // expected-error {{use of undeclared identifier 'node'}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r294009 - clang-format: [JS] Fix bugs in parsing and aligning template strings.
Author: djasper Date: Fri Feb 3 08:32:38 2017 New Revision: 294009 URL: http://llvm.org/viewvc/llvm-project?rev=294009&view=rev Log: clang-format: [JS] Fix bugs in parsing and aligning template strings. Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestJS.cpp Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=294009&r1=294008&r2=294009&view=diff == --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Fri Feb 3 08:32:38 2017 @@ -837,8 +837,8 @@ unsigned ContinuationIndenter::moveState } moveStatePastFakeLParens(State, Newline); - moveStatePastScopeOpener(State, Newline); moveStatePastScopeCloser(State); + moveStatePastScopeOpener(State, Newline); moveStatePastFakeRParens(State); if (Current.isStringLiteral() && State.StartOfStringLiteral == 0) @@ -1060,7 +1060,7 @@ void ContinuationIndenter::moveStatePast // If we encounter a closing ), ], } or >, we can remove a level from our // stacks. if (State.Stack.size() > 1 && - (Current.isOneOf(tok::r_paren, tok::r_square) || + (Current.isOneOf(tok::r_paren, tok::r_square, TT_TemplateString) || (Current.is(tok::r_brace) && State.NextToken != State.Line->First) || State.NextToken->is(TT_TemplateCloser))) State.Stack.pop_back(); Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=294009&r1=294008&r2=294009&view=diff == --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Feb 3 08:32:38 2017 @@ -1445,7 +1445,9 @@ public: // At the end of the line or when an operator with higher precedence is // found, insert fake parenthesis and return. - if (!Current || (Current->closesScope() && Current->MatchingParen) || + if (!Current || + (Current->closesScope() && + (Current->MatchingParen || Current->is(TT_TemplateString))) || (CurrentPrecedence != -1 && CurrentPrecedence < Precedence) || (CurrentPrecedence == prec::Conditional && Precedence == prec::Assignment && Current->is(tok::colon))) { Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=294009&r1=294008&r2=294009&view=diff == --- cfe/trunk/unittests/Format/FormatTestJS.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Feb 3 08:32:38 2017 @@ -1399,6 +1399,10 @@ TEST_F(FormatTestJS, TemplateStrings) { " a:${aaa.a} `;", "var f = `a:${aaa. a} \n" " a:${ aaa. a} `;"); + verifyFormat("var x = someFunction(`${})`) //\n" + ".oon();"); + verifyFormat("var x = someFunction(`${}${a( //\n" + " a)})`);"); } TEST_F(FormatTestJS, TemplateStringMultiLineExpression) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32
yaron.keren added a comment. Hiding these two include dirs removes many headers. Most has clang equivalents but not all of them. For example quadmath.h is only there, and without the include path programs using it will fail to compile. The reason mingw limits.h isn't used in your example is that clang version of limits.h specifically disables it: /* The system's limits.h may, in turn, try to #include_next GCC's limits.h. Avert this #include_next madness. */ #if defined __GNUC__ && !defined _GCC_LIMITS_H_ #define _GCC_LIMITS_H_ #endif so the solution may be stop disablng it... say #if defined __GNUC__ && !defined _GCC_LIMITS_H_ && !defined __MINGW32__ #define _GCC_LIMITS_H_ #endif https://reviews.llvm.org/D29464 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1
Anastasia added a comment. @arsenm, do you think you could complete the review? https://reviews.llvm.org/D29038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32
mati865 added a comment. I know why mingw-w64 limits.h weren't used (check first comment). At first I was using this hack: diff -urN clang.orig/3.9.0/include/limits.h clang/3.9.0/include/limits.h --- clang.orig/3.9.0/include/limits.h 2016-09-26 22:29:13.496441000 +0200 +++ clang/3.9.0/include/limits.h 2016-09-26 22:29:34.189625100 +0200 @@ -28,7 +28,7 @@ /* The system's limits.h may, in turn, try to #include_next GCC's limits.h. Avert this #include_next madness. */ #if defined __GNUC__ && !defined _GCC_LIMITS_H_ -#define _GCC_LIMITS_H_ +#define _LIMITS_H___ #endif /* System headers include a number of constants from POSIX in . It was working (for limiths.h at least) but disabling usage of GCC includes made it behaving more like on Linux. And since quadmath.h doesn't work on Linux with Clang I don't see issue. https://reviews.llvm.org/D29464 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
filcab added a comment. Why the switch to `if` instead of a fully-covered switch/case? In https://reviews.llvm.org/D29369#664426, @vsk wrote: > In https://reviews.llvm.org/D29369#664366, @regehr wrote: > > > Out of curiosity, how many of these superfluous checks are not subsequently > > eliminated by InstCombine? > > > I don't have numbers from a benchmark prepped. Here's what we get with the > 'ubsan-promoted-arith.cpp' test case from this patch: > > | Setup| # of overflow checks | > | unpatched, -O0 | 22 | > | unpatched, -O0 + instcombine | 7| > | patched, -O0 | 8| > | patched, -O0 + instcombine | 7| > > (There's a difference between the "patched, -O0" setup and the "patched, -O0 > + instcombine" setup because llvm figures out that the symbol 'a' is 0, and > gets rid of an addition that way.) > > At least for us, this patch is still worthwhile, because our use case is `-O0 > -fsanitized=undefined`. Also, this makes less work for instcombine, but I > haven't measured the compile-time effect. Probably running mem2reg and others before instcombine would make it elide more checks. But if you're using -O0 anyway, I guess this would help anyway. Comment at: test/CodeGen/ubsan-promoted-arith.cpp:56 + +// Note: -SHRT_MIN * -SHRT_MIN can overflow. +// Nit: Maybe `USHRT_MAX * USHRT_MAX` is more understandable? Comment at: test/CodeGen/ubsan-promoted-arith.cpp:99 +// CHECK-LABEL: define signext i8 @_Z4rem3 +// rdar30301609: ubsan_handle_divrem_overflow +char rem3(int i, char c) { return i % c; } Maybe put the rdar ID next to the FIXME? Like this it looks like you might have written that instead of `CHECK` by mistake. https://reviews.llvm.org/D29369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
alexfh added a comment. In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote: > Before clang-tidy came into existence the guidelines were very clear. One > should write a clang warning if the diagnostic: > > - can be implemented without path-insensitive analysis (including > flow-sensitive) > - is checking for language rules (not specific API misuse) > > In my view, this should still be the rule going forward because compiler > warnings are most effective in reaching users. > > The Clang Static Analyzer used to be the place for all other diagnostics. > Most of the checkers it contains rely on path-sensitive analysis. Note that > one might catch the same bug with flow-sensitive as well as path-sensitive > analysis. So in some of those cases, we have both warnings as well as > analyzer checkers finding the same type of user error. However, the checkers > can catch more bugs since they are path-sensitive. The analyzer also has > several flow-sensitive/ AST matching checkers. Those checks could not have > been written as warnings mainly because they check for APIs, which are not > part of the language. > > My understanding is that clang-tidy supports fixits, which the clang static > analyzer currently does not support. However, there could be other benefits > to placing not path-sensitive checks there as well. I am not sure. I've also > heard opinions that it's more of a linter tool, so the user expectations > could be different. > > > Even right now there are clang-tidy checks that finds subset of alpha > > checks, but probably having lower false positive rate. > > Again, alpha checks are unfinished work, so we should not use them as > examples of checkers that have false positives. Some of them are research > projects and should probably be deleted. I tried to come up with some advice on where checks should go in http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check: | Clang diagnostic: if the check is generic enough, targets code patterns that most probably are bugs (rather than style or readability issues), can be implemented effectively and with extremely low false positive rate, it may make a good Clang diagnostic. | | Clang static analyzer check: if the check requires some sort of control flow analysis, it should probably be implemented as a static analyzer check. | | clang-tidy check is a good choice for linter-style checks, checks that are related to a certain coding style, checks that address code readability, etc. | There's no doubt path-sensitive checks should go to Static Analyzer, since it provides all the infrastructure for path-sensitive analyses. Whatever meets the criteria for a Clang diagnostic should be a diagnostic. Whatever needs automated fixes (and can be implemented on AST or preprocessor level) should go to clang-tidy. But there's still a large set of analyses that don't clearly fall into one of the categories above and can be implemented both in Clang Static Analyzer and in clang-tidy. Currently there are no firm rules about those, only recommendations on the clang-tidy page (quoted above). We might need to agree upon some reasonable guidelines, though. For example, IMO, AST-based analyses make more sense in clang-tidy for a few reasons: 1. They usually are easier expressible in terms of AST matchers, and clang-tidy allows to use AST matchers with less boilerplate. 2. Clang Static Analyzer is linked into clang, where AST matchers are undesired, since they tend to blow the size of binary significantly. 3. It's more consistent to keep all similar analyses together, it simplifies search for already implemented similar functionality and code reviews. Flow-sensitive analyses (that don't need any path-sensitivity) seem to be equally suitable for SA and clang-tidy (unless I'm missing something), so I don't feel strongly as to where they should go. WDYT? Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29393: [clang-tidy] Don't warn about call to unresolved operator*
idlecode added inline comments. Comment at: clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp:63 +cxxOperatorCallExpr(argumentCountIs(1), +callee(unresolvedLookupExpr()), +hasArgument(0, cxxThisExpr(; Seems that it will catch all unary operators with ##this## as first argument. e.g. in case `operator-` is defined somewhere, check will not warn about returning `-this`. Do you think adding `hasOverloadedOperatorName("*")` for such abstract case is worth it? https://reviews.llvm.org/D29393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin
dim added a comment. In https://reviews.llvm.org/D28213#639397, @hfinkel wrote: > LGTM. This seems consistent with what GCC does. On x86 it also sets > __GCC_ATOMIC_LLONG_LOCK_FREE to 2. Hmm, unfortunately on i386-freebsd, it does not: $ gcc6 -v Using built-in specs. COLLECT_GCC=gcc6 COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc6/gcc/i386-portbld-freebsd12.0/6.3.0/lto-wrapper Target: i386-portbld-freebsd12.0 Configured with: /wrkdirs/usr/ports/lang/gcc6/work/gcc-6.3.0/configure --disable-multilib --disable-bootstrap --disable-nls --enable-gnu-indirect-function --libdir=/usr/local/lib/gcc6 --libexecdir=/usr/local/libexec/gcc6 --program-suffix=6 --with-as=/usr/local/bin/as --with-gmp=/usr/local --with-gxx-include-dir=/usr/local/lib/gcc6/include/c++/ --with-ld=/usr/local/bin/ld --with-pkgversion='FreeBSD Ports Collection' --with-system-zlib --disable-libgcj --enable-languages=c,c++,objc,fortran --prefix=/usr/local --localstatedir=/var --mandir=/usr/local/man --infodir=/usr/local/info/gcc6 --build=i386-portbld-freebsd12.0 Thread model: posix gcc version 6.3.0 (FreeBSD Ports Collection) $ gcc6 -dM -E -x c /dev/null | grep __GCC_ATOMIC_LLONG_LOCK_FREE #define __GCC_ATOMIC_LLONG_LOCK_FREE 1 $ clang -v FreeBSD clang version 4.0.0 (branches/release_40 293807) (based on LLVM 4.0.0) Target: i386-unknown-freebsd12.0 Thread model: posix InstalledDir: /usr/bin $ clang -dM -E -x c /dev/null | grep __GCC_ATOMIC_LLONG_LOCK_FREE #define __GCC_ATOMIC_LLONG_LOCK_FREE 2 I don't think FreeBSD has lock-free 64 bit atomic operations on 32-bit x86. IIRC we already had some trouble before with clang emitting libcalls to `__atomic_fetch_add_8` and friends, which then lead to linking errors. See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216745, where this is now occurring with boost. Repository: rL LLVM https://reviews.llvm.org/D28213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29480: C++11 Compatibility - OpenMP constant expressions
This revision was automatically updated to reflect the committed changes. Closed by commit rL294025: [Lit Test] Make tests C++11 compatible - OpenMP constant expressions (authored by lcharles). Changed prior to commit: https://reviews.llvm.org/D29480?vs=86925&id=86988#toc Repository: rL LLVM https://reviews.llvm.org/D29480 Files: cfe/trunk/test/OpenMP/distribute_collapse_messages.cpp cfe/trunk/test/OpenMP/ordered_messages.cpp cfe/trunk/test/OpenMP/target_parallel_for_collapse_messages.cpp cfe/trunk/test/OpenMP/target_parallel_for_ordered_messages.cpp Index: cfe/trunk/test/OpenMP/target_parallel_for_ordered_messages.cpp === --- cfe/trunk/test/OpenMP/target_parallel_for_ordered_messages.cpp +++ cfe/trunk/test/OpenMP/target_parallel_for_ordered_messages.cpp @@ -1,9 +1,14 @@ // RUN: %clang_cc1 -verify -fopenmp %s +// RUN: %clang_cc1 -verify -fopenmp -std=c++98 %s +// RUN: %clang_cc1 -verify -fopenmp -std=c++11 %s void foo() { } bool foobool(int argc) { +#if __cplusplus >= 201103L +// expected-note@-2 4 {{declared here}} +#endif return argc; } @@ -36,6 +41,9 @@ #pragma omp target parallel for ordered((ST > 0) ? 1 + ST : 2) // expected-note 2 {{as specified in 'ordered' clause}} for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i - ST]; // expected-error 2 {{expected 2 for loops after '#pragma omp target parallel for', but found only 1}} +#if __cplusplus >= 201103L +// expected-note@+5 2 {{non-constexpr function 'foobool' cannot be used in a constant expression}} +#endif // expected-error@+3 2 {{directive '#pragma omp target parallel for' cannot contain more than one 'ordered' clause}} // expected-error@+2 2 {{argument to 'ordered' clause must be a strictly positive integer value}} // expected-error@+1 2 {{expression is not an integral constant expression}} @@ -45,7 +53,11 @@ #pragma omp target parallel for ordered(S) // expected-error {{'S' does not refer to a value}} for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i - ST]; -// expected-error@+1 2 {{expression is not an integral constant expression}} +#if __cplusplus >= 201103L + // expected-error@+4 2 {{integral constant expression must have integral or unscoped enumeration type, not 'char *'}} +#else + // expected-error@+2 2 {{expression is not an integral constant expression}} +#endif #pragma omp target parallel for ordered(argv[1] = 2) // expected-error {{expected ')'}} expected-note {{to match this '('}} for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i - ST]; @@ -76,9 +88,15 @@ #pragma omp target parallel for ordered(2 + 2)) // expected-warning {{extra tokens at the end of '#pragma omp target parallel for' are ignored}} expected-note {{as specified in 'ordered' clause}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i - 4];// expected-error {{expected 4 for loops after '#pragma omp target parallel for', but found only 1}} +#if __cplusplus >= 201103L +// expected-note@+2 {{non-constexpr function 'foobool' cannot be used in a constant expression}} +#endif #pragma omp target parallel for ordered(foobool(1) > 0 ? 1 : 2) // expected-error {{expression is not an integral constant expression}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i - 4]; +#if __cplusplus >= 201103L +// expected-note@+5 {{non-constexpr function 'foobool' cannot be used in a constant expression}} +#endif // expected-error@+3 {{expression is not an integral constant expression}} // expected-error@+2 2 {{directive '#pragma omp target parallel for' cannot contain more than one 'ordered' clause}} // expected-error@+1 2 {{argument to 'ordered' clause must be a strictly positive integer value}} @@ -88,7 +106,11 @@ #pragma omp target parallel for ordered(S1) // expected-error {{'S1' does not refer to a value}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i - 4]; -// expected-error@+1 {{expression is not an integral constant expression}} +#if __cplusplus >= 201103L + // expected-error@+4 {{integral constant expression must have integral or unscoped enumeration type, not 'char *'}} +#else + // expected-error@+2 {{expression is not an integral constant expression}} +#endif #pragma omp target parallel for ordered(argv[1] = 2) // expected-error {{expected ')'}} expected-note {{to match this '('}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i - 4]; Index: cfe/trunk/test/OpenMP/ordered_messages.cpp === --- cfe/trunk/test/OpenMP/ordered_messages.cpp +++ cfe/trunk/test/OpenMP/ordered_messages.cpp @@ -1,4 +1,6 @@ // RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s +// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -std=c++98 -o - %s +// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -std=c++11 -o - %s int foo(); @@ -123,6 +1
r294025 - [Lit Test] Make tests C++11 compatible - OpenMP constant expressions
Author: lcharles Date: Fri Feb 3 12:58:34 2017 New Revision: 294025 URL: http://llvm.org/viewvc/llvm-project?rev=294025&view=rev Log: [Lit Test] Make tests C++11 compatible - OpenMP constant expressions C++11 introduced constexpr, hence the change in diagnostics. Differential Revision: https://reviews.llvm.org/D29480 Modified: cfe/trunk/test/OpenMP/distribute_collapse_messages.cpp cfe/trunk/test/OpenMP/ordered_messages.cpp cfe/trunk/test/OpenMP/target_parallel_for_collapse_messages.cpp cfe/trunk/test/OpenMP/target_parallel_for_ordered_messages.cpp Modified: cfe/trunk/test/OpenMP/distribute_collapse_messages.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_collapse_messages.cpp?rev=294025&r1=294024&r2=294025&view=diff == --- cfe/trunk/test/OpenMP/distribute_collapse_messages.cpp (original) +++ cfe/trunk/test/OpenMP/distribute_collapse_messages.cpp Fri Feb 3 12:58:34 2017 @@ -1,8 +1,13 @@ // RUN: %clang_cc1 -verify -fopenmp %s +// RUN: %clang_cc1 -verify -fopenmp -std=c++98 %s +// RUN: %clang_cc1 -verify -fopenmp -std=c++11 %s void foo() { } +#if __cplusplus >= 201103L + // expected-note@+2 4 {{declared here}} +#endif bool foobool(int argc) { return argc; } @@ -29,6 +34,9 @@ T tmain(T argc, S **argv) { //expected-n for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i-ST]; #pragma omp distribute collapse ((ST > 0) ? 1 + ST : 2) // expected-note 2 {{as specified in 'collapse' clause}} for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i-ST]; // expected-error 2 {{expected 2 for loops after '#pragma omp distribute', but found only 1}} +#if __cplusplus >= 201103L + // expected-note@+5 2 {{non-constexpr function 'foobool' cannot be used in a constant expression}} +#endif // expected-error@+3 2 {{directive '#pragma omp distribute' cannot contain more than one 'collapse' clause}} // expected-error@+2 2 {{argument to 'collapse' clause must be a strictly positive integer value}} // expected-error@+1 2 {{expression is not an integral constant expression}} @@ -36,7 +44,11 @@ T tmain(T argc, S **argv) { //expected-n for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i-ST]; #pragma omp distribute collapse (S) // expected-error {{'S' does not refer to a value}} for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i-ST]; - // expected-error@+1 2 {{expression is not an integral constant expression}} +#if __cplusplus <= 199711L + // expected-error@+4 2 {{expression is not an integral constant expression}} +#else + // expected-error@+2 2 {{integral constant expression must have integral or unscoped enumeration type, not 'char *'}} +#endif #pragma omp distribute collapse (argv[1]=2) // expected-error {{expected ')'}} expected-note {{to match this '('}} for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i-ST]; #pragma omp distribute collapse (1) @@ -59,8 +71,14 @@ int main(int argc, char **argv) { for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i-4]; // expected-error {{expected 4 for loops after '#pragma omp distribute', but found only 1}} #pragma omp distribute collapse (2+2)) // expected-warning {{extra tokens at the end of '#pragma omp distribute' are ignored}} expected-note {{as specified in 'collapse' clause}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i-4]; // expected-error {{expected 4 for loops after '#pragma omp distribute', but found only 1}} +#if __cplusplus >= 201103L + // expected-note@+2 {{non-constexpr function 'foobool' cannot be used in a constant expression}} +#endif #pragma omp distribute collapse (foobool(1) > 0 ? 1 : 2) // expected-error {{expression is not an integral constant expression}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i-4]; +#if __cplusplus >= 201103L + // expected-note@+5 {{non-constexpr function 'foobool' cannot be used in a constant expression}} +#endif // expected-error@+3 {{expression is not an integral constant expression}} // expected-error@+2 2 {{directive '#pragma omp distribute' cannot contain more than one 'collapse' clause}} // expected-error@+1 2 {{argument to 'collapse' clause must be a strictly positive integer value}} @@ -68,7 +86,11 @@ int main(int argc, char **argv) { for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i-4]; #pragma omp distribute collapse (S1) // expected-error {{'S1' does not refer to a value}} for (int i = 4; i < 12; i++) argv[0][i] = argv[0][i] - argv[0][i-4]; - // expected-error@+1 {{expression is not an integral constant expression}} +#if __cplusplus <= 199711L + // expected-error@+4 {{expression is not an integral constant expression}} +#else + // expected-error@+2 {{integral constant expression must have integral or unscoped enumeration type, not 'char *'}} +#endif #pragma omp distrib
[PATCH] D29267: [clang-tidy] safety-no-assembler
idlecode added a comment. Standard you linked mentions portability as the reason inline assembler should be avoided. Should it really be named 'safety'? https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r294026 - [OpenMP] Add missing regression test for pragma distribute, clause firstprivate
Author: cbertol Date: Fri Feb 3 13:09:16 2017 New Revision: 294026 URL: http://llvm.org/viewvc/llvm-project?rev=294026&view=rev Log: [OpenMP] Add missing regression test for pragma distribute, clause firstprivate https://reviews.llvm.org/D28243 The regression test was missing from the previous already accepted patch. Added: cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp Added: cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp?rev=294026&view=auto == --- cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp (added) +++ cfe/trunk/test/OpenMP/distribute_firstprivate_codegen.cpp Fri Feb 3 13:09:16 2017 @@ -0,0 +1,382 @@ +// RUN: %clang_cc1 -DLAMBDA -verify -fopenmp -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix LAMBDA --check-prefix LAMBDA-64 +// RUN: %clang_cc1 -DLAMBDA -fopenmp -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s +// RUN: %clang_cc1 -DLAMBDA -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix LAMBDA --check-prefix LAMBDA-64 +// RUN: %clang_cc1 -DLAMBDA -verify -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix LAMBDA --check-prefix LAMBDA-32 +// RUN: %clang_cc1 -DLAMBDA -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s +// RUN: %clang_cc1 -DLAMBDA -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix LAMBDA --check-prefix LAMBDA-32 + +// RUN: %clang_cc1 -verify -fopenmp -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64 +// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s +// RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64 +// RUN: %clang_cc1 -verify -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 +// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s +// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 +// expected-no-diagnostics +#ifndef HEADER +#define HEADER + +template +struct S { + T f; + S(T a) : f(a) {} + S() : f() {} + operator T() { return T(); } + ~S() {} +}; + +// CHECK: [[S_FLOAT_TY:%.+]] = type { float } +// CHECK: [[S_INT_TY:%.+]] = type { i{{[0-9]+}} } +template +T tmain() { + S test; + T t_var = T(); + T vec[] = {1, 2}; + S s_arr[] = {1, 2}; + S &var = test; + #pragma omp target + #pragma omp teams +#pragma omp distribute firstprivate(t_var, vec, s_arr, s_arr, var, var) + for (int i = 0; i < 2; ++i) { +vec[i] = t_var; +s_arr[i] = var; + } + return T(); +} + +int main() { + static int svar; + volatile double g; + volatile double &g1 = g; + + #ifdef LAMBDA + // LAMBDA-LABEL: @main + // LAMBDA: call{{.*}} void [[OUTER_LAMBDA:@.+]]( + [&]() { +static float sfvar; +// LAMBDA: define{{.*}} internal{{.*}} void [[OUTER_LAMBDA]]( +// LAMBDA: call i{{[0-9]+}} @__tgt_target_teams( +// LAMBDA: call void [[OFFLOADING_FUN:@.+]]( + +// LAMBDA: define{{.+}} void [[OFFLOADING_FUN]]( +// LAMBDA: call {{.*}}void {{.+}} @__kmpc_fork_teams({{.+}}, {{.+}}, {{.+}}* [[OMP_OUTLINED:@.+]] to {{.+}}) +#pragma omp target +#pragma omp teams +#pragma omp distribute firstprivate(g, g1, svar, sfvar) +for (int i = 0; i < 2; ++i) { + // LAMBDA-64: define{{.*}} internal{{.*}} void [[OMP_OUTLINED]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, i{{[0-9]+}} [[G_IN:%.+]], i{{[0-9]+}} [[G1_IN:%.+]], i{{[0-9]+}} [[SVAR_IN:%.+]], i{{[0-9]+}} [[SFVAR_IN:%.+]]) + // LAMBDA-32: define internal{{.*}} void [[OMP_OUTLINED]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, double*{{.*}} [[G_IN:%.+]], i{{[0-9]+}} [[G1_IN:%.+]], i{{[0-9]+}} [[SVAR_IN:%.+]], i{{[0-9]+}} [[SFVAR_IN:%.+]]) + // Private alloca's for
[PATCH] D28243: [OpenMP] Add missing regression test for pragma distribute, clause firstprivate
carlo.bertolli closed this revision. carlo.bertolli added a comment. Committed to r294026 https://reviews.llvm.org/D28243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20710: Lit C++11 Compatibility Patch #9
tigerleapgorge updated this revision to Diff 86994. tigerleapgorge added a comment. Update again, remove 4 OpenMP tests here because they have been checked in https://reviews.llvm.org/rL294025 under review https://reviews.llvm.org/D29480. 13 tests left. https://reviews.llvm.org/D20710 Files: test/CodeGenCXX/debug-info-use-after-free.cpp test/CodeGenCXX/dynamic-cast-hint.cpp test/SemaCXX/i-c-e-cxx.cpp test/SemaCXX/implicit-virtual-member-functions.cpp test/SemaCXX/new-delete.cpp test/SemaCXX/no-wchar.cpp test/SemaCXX/virtual-member-functions-key-function.cpp test/SemaCXX/warn-bool-conversion.cpp test/SemaCXX/zero-length-arrays.cpp test/SemaTemplate/instantiate-c99.cpp test/SemaTemplate/temp_explicit.cpp test/SemaTemplate/value-dependent-null-pointer-constant.cpp test/SemaTemplate/virtual-member-functions.cpp Index: test/SemaTemplate/virtual-member-functions.cpp === --- test/SemaTemplate/virtual-member-functions.cpp +++ test/SemaTemplate/virtual-member-functions.cpp @@ -1,5 +1,9 @@ // RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++98 %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++11 %s // RUN: %clang_cc1 -triple %ms_abi_triple -DMSABI -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple %ms_abi_triple -DMSABI -fsyntax-only -std=c++98 -verify %s +// RUN: %clang_cc1 -triple %ms_abi_triple -DMSABI -fsyntax-only -std=c++11 -verify %s namespace PR5557 { template struct A { @@ -76,34 +80,76 @@ } namespace PR7114 { - class A { virtual ~A(); }; // expected-note{{declared private here}} + class A { virtual ~A(); }; +#if __cplusplus <= 199711L + // expected-note@-2{{declared private here}} +#else + // expected-note@-4 3 {{overridden virtual function is here}} +#endif template class B { public: -class Inner : public A { }; // expected-error{{base class 'PR7114::A' has private destructor}} +class Inner : public A { }; +#if __cplusplus <= 199711L +// expected-error@-2{{base class 'PR7114::A' has private destructor}} +#else +// expected-error@-4 2 {{deleted function '~Inner' cannot override a non-deleted function}} +// expected-note@-5 2 {{destructor of 'Inner' is implicitly deleted because base class 'PR7114::A' has an inaccessible destructor}} +#ifdef MSABI +// expected-note@-7 1 {{destructor of 'Inner' is implicitly deleted because base class 'PR7114::A' has an inaccessible destructor}} +#endif +#endif + static Inner i; static const unsigned value = sizeof(i) == 4; +#if __cplusplus >= 201103L +// expected-note@-2 {{in instantiation of member class 'PR7114::B::Inner' requested here}} +// expected-note@-3 {{in instantiation of member class 'PR7114::B::Inner' requested here}} +#endif }; int f() { return B::value; } +#if __cplusplus >= 201103L +// expected-note@-2 {{in instantiation of template class 'PR7114::B' requested here}} +#endif #ifdef MSABI - void test_typeid(B::Inner bfi) { // expected-note{{implicit destructor}} + void test_typeid(B::Inner bfi) { +#if __cplusplus <= 199711L +// expected-note@-2 {{implicit destructor}} +#else +// expected-error@-4 {{attempt to use a deleted function}} +// expected-note@-5 {{in instantiation of template class 'PR7114::B' requested here}} +#endif + (void)typeid(bfi); #else void test_typeid(B::Inner bfi) { -(void)typeid(bfi); // expected-note{{implicit destructor}} +#if __cplusplus >= 201103L +// expected-note@-2 {{in instantiation of template class 'PR7114::B' requested here}} +#endif +(void)typeid(bfi); +#if __cplusplus <= 199711L +// expected-note@-2 {{implicit destructor}} +#endif #endif } template struct X : A { +#if __cplusplus >= 201103L +// expected-error@-2 {{deleted function '~X' cannot override a non-deleted function}} +// expected-note@-3 {{destructor of 'X' is implicitly deleted because base class 'PR7114::A' has an inaccessible destructor}} +#endif void f() { } }; void test_X(X &xi, X &xf) { xi.f(); +#if __cplusplus >= 201103L +// expected-note@-2 {{in instantiation of template class 'PR7114::X' requested here}} +#endif } } Index: test/SemaTemplate/value-dependent-null-pointer-constant.cpp === --- test/SemaTemplate/value-dependent-null-pointer-constant.cpp +++ test/SemaTemplate/value-dependent-null-pointer-constant.cpp @@ -1,17 +1,30 @@ -// RUN: %clang_cc1 -fsyntax-only %s +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s template struct X0 { const char *f0(bool Cond) { return Cond? "honk" : N; +#if __cplusplus >= 201103L +// expected-error@-2 {{incompatible operand types ('const char *' and 'int')}} +#else +// expected-no-diagnostics +#endif } co
[PATCH] D29506: [OpenMP] Teams reduction on the NVPTX device.
arpith-jacob added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:524 + + static bool classof(const CGOpenMPRuntime *RT) { +return RT->getKind() == RK_HOST; This is required to cast to the NVPTX runtime in a static function as follows; CGOpenMPRuntimeNVPTX &RT = cast(CGM.getOpenMPRuntime()); Comment at: lib/CodeGen/CGOpenMPRuntime.h:975 + bool WithNowait, bool SimpleReduction, + OpenMPDirectiveKind ReductionKind); NVPTX requires this parameter because the reduction implementation for 'parallel', 'simd', and 'teams' are different. This is required for correctness and performance. https://reviews.llvm.org/D29506 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1
arsenm accepted this revision. arsenm added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/SemaOpenCL/logical-ops.cl:1-3 +// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu +// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu + Anastasia wrote: > Anastasia wrote: > > arsenm wrote: > > > arsenm wrote: > > > > Should this have a 2.0 run line for good measure? > > > 1.0 too I suppose > > Sure! > Apparently CL1.0 is not supported! I missed that That sounds like a bug? https://reviews.llvm.org/D29038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29393: [clang-tidy] Don't warn about call to unresolved operator*
malcolm.parsons added inline comments. Comment at: clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp:63 +cxxOperatorCallExpr(argumentCountIs(1), +callee(unresolvedLookupExpr()), +hasArgument(0, cxxThisExpr(; idlecode wrote: > Seems that it will catch all unary operators with ##this## as first argument. > e.g. in case `operator-` is defined somewhere, check will not warn about > returning `-this`. > Do you think adding `hasOverloadedOperatorName("*")` for such abstract case > is worth it? I'm just trying to suppress them warning from the template. The operator can be checked properly in an instantiation. https://reviews.llvm.org/D29393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin
mgorny added a comment. In https://reviews.llvm.org/D28213#665967, @dim wrote: > I don't think FreeBSD has lock-free 64 bit atomic operations on 32-bit x86. > IIRC we already had some trouble before with clang emitting libcalls to > `__atomic_fetch_add_8` and friends, which then lead to linking errors. > > See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216745, where this is > now occurring with boost. Could you try to figure out what's the cause for that discrepancy? What's the value of `__atomic_always_lock_free(sizeof(long long), 0)` for gcc and clang? Repository: rL LLVM https://reviews.llvm.org/D28213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r293043 - [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested blocks.
Ping? On Wed, Feb 1, 2017 at 11:00 AM, Hans Wennborg wrote: > If Anna is Ok with it, I'm fine with merging. > > Thanks, > Hans > > On Wed, Feb 1, 2017 at 10:29 AM, Artem Dergachev wrote: >> Hans, >> >> This is a fixed and tested version of the previously-merged-and-reverted >> r292800, do we still have time to land this into 4.0? >> >> Thanks, >> Artem. >> >> >> On 1/25/17 1:21 PM, Artem Dergachev via cfe-commits wrote: >>> >>> Author: dergachev >>> Date: Wed Jan 25 04:21:45 2017 >>> New Revision: 293043 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=293043&view=rev >>> Log: >>> [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested >>> blocks. >>> >>> This is an attempt to avoid new false positives caused by the reverted >>> r292800, >>> however the scope of the fix is significantly reduced - some variables are >>> still >>> in incorrect memory spaces. >>> >>> Relevant test cases added. >>> >>> rdar://problem/30105546 >>> rdar://problem/30156693 >>> Differential revision: https://reviews.llvm.org/D28946 >>> >>> Added: >>> cfe/trunk/test/Analysis/null-deref-static.m >>> Modified: >>> cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp >>> cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp >>> cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp >>> cfe/trunk/test/Analysis/dispatch-once.m >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp Wed Jan 25 >>> 04:21:45 2017 >>> @@ -94,11 +94,18 @@ void MacOSXAPIChecker::CheckDispatchOnce >>> bool SuggestStatic = false; >>> os << "Call to '" << FName << "' uses"; >>> if (const VarRegion *VR = dyn_cast(RB)) { >>> +const VarDecl *VD = VR->getDecl(); >>> +// FIXME: These should have correct memory space and thus should be >>> filtered >>> +// out earlier. This branch only fires when we're looking from a >>> block, >>> +// which we analyze as a top-level declaration, onto a static local >>> +// in a function that contains the block. >>> +if (VD->isStaticLocal()) >>> + return; >>> // We filtered out globals earlier, so it must be a local variable >>> // or a block variable which is under UnknownSpaceRegion. >>> if (VR != R) >>> os << " memory within"; >>> -if (VR->getDecl()->hasAttr()) >>> +if (VD->hasAttr()) >>> os << " the block variable '"; >>> else >>> os << " the local variable '"; >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Jan 25 04:21:45 >>> 2017 >>> @@ -816,9 +816,11 @@ const VarRegion* MemRegionManager::getVa >>> const StackFrameContext *STC = V.get(); >>> -if (!STC) >>> +if (!STC) { >>> + // FIXME: Assign a more sensible memory space to static locals >>> + // we see from within blocks that we analyze as top-level >>> declarations. >>> sReg = getUnknownRegion(); >>> -else { >>> +} else { >>> if (D->hasLocalStorage()) { >>> sReg = isa(D) || isa(D) >>> ? static_cast>> MemRegion*>(getStackArgumentsRegion(STC)) >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Jan 25 04:21:45 >>> 2017 >>> @@ -1849,6 +1849,8 @@ SVal RegionStoreManager::getBindingForVa >>> // Function-scoped static variables are default-initialized to 0; >>> if they >>> // have an initializer, it would have been processed by now. >>> +// FIXME: This is only true when we're starting analysis from main(). >>> +// We're losing a lot of coverage here. >>> if (isa(MS)) >>> return svalBuilder.makeZeroVal(T); >>> >>> Modified: cfe/trunk/test/Analysis/dispatch-once.m >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dispatch-once.m?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/test/Analysis/dis
[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin
dim added a comment. In https://reviews.llvm.org/D28213#666196, @mgorny wrote: > In https://reviews.llvm.org/D28213#665967, @dim wrote: > > > I don't think FreeBSD has lock-free 64 bit atomic operations on 32-bit x86. > > IIRC we already had some trouble before with clang emitting libcalls to > > `__atomic_fetch_add_8` and friends, which then lead to linking errors. > > > > See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216745, where this is > > now occurring with boost. > > > Could you try to figure out what's the cause for that discrepancy? I think the discrepancy is that many 32-bit Linuxes target at least i686 by default, and then you have CMPXCHG8B most of the time (apparently not always, but nobody seems to care): $ uname -srm Linux 3.16.0-4-686-pae i686 $ gcc -dM -E -x c /dev/null | grep LLONG_LOCK #define __GCC_ATOMIC_LLONG_LOCK_FREE 2 However, 32-bit FreeBSD targets i486 by default: $ uname -srm FreeBSD 12.0-CURRENT i386 $ gcc -dM -E -x c /dev/null | grep LLONG_LOCK #define __GCC_ATOMIC_LLONG_LOCK_FREE 1 $ clang-3.9.1 -dM -E -x c /dev/null | grep LLONG_LOCK #define __GCC_ATOMIC_LLONG_LOCK_FREE 1 $ clang-4.0.0 -dM -E -x c /dev/null | grep LLONG_LOCK #define __GCC_ATOMIC_LLONG_LOCK_FREE 2 Note that even on 32-bit Linux you get 1 for `__GCC_ATOMIC_LLONG_LOCK_FREE`, if you target i486: $ uname -srm Linux 3.16.0-4-686-pae i686 $ gcc -march=i486 -dM -E -x c /dev/null | grep LLONG_LOCK #define __GCC_ATOMIC_LLONG_LOCK_FREE 1 > What's the value of `__atomic_always_lock_free(sizeof(long long), 0)` for gcc > and clang? For gcc, it is always 0, for clang (I tested 3.4.1 through 4.0.0) it is always 1. Maybe that was always incorrect on 32-bit FreeBSD, then? Repository: rL LLVM https://reviews.llvm.org/D28213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r294008 - [Sema][ObjC++] Typo correction should handle ivars and properties
IIUC, this isn't strictly fixing a regression from 3.9, but it looks like a pretty small diff. Richard, what do you think? On Fri, Feb 3, 2017 at 6:37 AM, Alex L wrote: > Hi Hans, > > Is there any chance we can merge this for 4.0? It fixed a nasty bug where > clang didn't catch invalid ObjC++ code during semantic analysis which led to > invalid object files or crashes in CodeGen. > > Cheers, > Alex > > On 3 February 2017 at 14:22, Alex Lorenz via cfe-commits > wrote: >> >> Author: arphaman >> Date: Fri Feb 3 08:22:33 2017 >> New Revision: 294008 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=294008&view=rev >> Log: >> [Sema][ObjC++] Typo correction should handle ivars and properties >> >> After r260016 and r260017 disabled typo correction for ivars and >> properties >> clang didn't report errors about unresolved identifier in the base of ivar >> and >> property ref expressions. This meant that clang invoked CodeGen on invalid >> AST >> which then caused a crash. >> >> This commit re-enables typo correction for ivars and properites, and fixes >> the >> PR25113 & PR26486 (that were originally fixed in r260017 and r260016) in a >> different manner by transforming the Objective-C ivar reference expression >> with >> 'IsFreeIvar' preserved. >> >> rdar://30310772 >> >> Modified: >> cfe/trunk/lib/Sema/SemaExprCXX.cpp >> cfe/trunk/lib/Sema/TreeTransform.h >> cfe/trunk/test/SemaObjCXX/typo-correction.mm >> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=294008&r1=294007&r2=294008&view=diff >> >> == >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb 3 08:22:33 2017 >> @@ -7250,14 +7250,6 @@ public: >> >>ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); } >> >> - ExprResult TransformObjCPropertyRefExpr(ObjCPropertyRefExpr *E) { >> -return Owned(E); >> - } >> - >> - ExprResult TransformObjCIvarRefExpr(ObjCIvarRefExpr *E) { >> -return Owned(E); >> - } >> - >>ExprResult Transform(Expr *E) { >> ExprResult Res; >> while (true) { >> >> Modified: cfe/trunk/lib/Sema/TreeTransform.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=294008&r1=294007&r2=294008&view=diff >> >> == >> --- cfe/trunk/lib/Sema/TreeTransform.h (original) >> +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Feb 3 08:22:33 2017 >> @@ -2982,16 +2982,17 @@ public: >>ExprResult RebuildObjCIvarRefExpr(Expr *BaseArg, ObjCIvarDecl *Ivar, >>SourceLocation IvarLoc, >>bool IsArrow, bool IsFreeIvar) >> { >> -// FIXME: We lose track of the IsFreeIvar bit. >> CXXScopeSpec SS; >> DeclarationNameInfo NameInfo(Ivar->getDeclName(), IvarLoc); >> -return getSema().BuildMemberReferenceExpr(BaseArg, >> BaseArg->getType(), >> - /*FIXME:*/IvarLoc, IsArrow, >> - SS, SourceLocation(), >> - >> /*FirstQualifierInScope=*/nullptr, >> - NameInfo, >> - /*TemplateArgs=*/nullptr, >> - /*S=*/nullptr); >> +ExprResult Result = getSema().BuildMemberReferenceExpr( >> +BaseArg, BaseArg->getType(), >> +/*FIXME:*/ IvarLoc, IsArrow, SS, SourceLocation(), >> +/*FirstQualifierInScope=*/nullptr, NameInfo, >> +/*TemplateArgs=*/nullptr, >> +/*S=*/nullptr); >> +if (IsFreeIvar && Result.isUsable()) >> + cast(Result.get())->setIsFreeIvar(IsFreeIvar); >> +return Result; >>} >> >>/// \brief Build a new Objective-C property reference expression. >> >> Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/typo-correction.mm?rev=294008&r1=294007&r2=294008&view=diff >> >> == >> --- cfe/trunk/test/SemaObjCXX/typo-correction.mm (original) >> +++ cfe/trunk/test/SemaObjCXX/typo-correction.mm Fri Feb 3 08:22:33 2017 >> @@ -21,3 +21,18 @@ public: >>self.m_prop2 = new ClassB(m_prop1); // expected-error {{use of >> undeclared identifier 'm_prop1'; did you mean '_m_prop1'?}} >> } >> @end >> + >> +// rdar://30310772 >> + >> +@interface InvalidNameInIvarAndPropertyBase >> +{ >> +@public >> + float _a; >> +} >> +@property float _b; >> +@end >> + >> +void invalidNameInIvarAndPropertyBase() { >> + float a = ((InvalidNameInIvarAndPropertyBase*)node)->_a; // >> expected-error {{use of undeclared identifier 'node'}} >> + float b = ((InvalidNameInIvarAndPropertyBase*)node)._b; // >> expected-error
[PATCH] D29267: [clang-tidy] safety-no-assembler
jbcoe added a comment. High Integrity C++ is often used as a standard for safety-critical systems. High Integrity C++ requires no assembler due to portability issues. Not my choice of wording. I plan to implement more of the High Integrity C++ checks, some of which are more obviously safety-related. https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r293043 - [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested blocks.
Fine with merging. Thank you! Anna. > On Feb 1, 2017, at 11:00 AM, Hans Wennborg wrote: > > If Anna is Ok with it, I'm fine with merging. > > Thanks, > Hans > > On Wed, Feb 1, 2017 at 10:29 AM, Artem Dergachev wrote: >> Hans, >> >> This is a fixed and tested version of the previously-merged-and-reverted >> r292800, do we still have time to land this into 4.0? >> >> Thanks, >> Artem. >> >> >> On 1/25/17 1:21 PM, Artem Dergachev via cfe-commits wrote: >>> >>> Author: dergachev >>> Date: Wed Jan 25 04:21:45 2017 >>> New Revision: 293043 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=293043&view=rev >>> Log: >>> [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested >>> blocks. >>> >>> This is an attempt to avoid new false positives caused by the reverted >>> r292800, >>> however the scope of the fix is significantly reduced - some variables are >>> still >>> in incorrect memory spaces. >>> >>> Relevant test cases added. >>> >>> rdar://problem/30105546 >>> rdar://problem/30156693 >>> Differential revision: https://reviews.llvm.org/D28946 >>> >>> Added: >>> cfe/trunk/test/Analysis/null-deref-static.m >>> Modified: >>> cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp >>> cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp >>> cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp >>> cfe/trunk/test/Analysis/dispatch-once.m >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp Wed Jan 25 >>> 04:21:45 2017 >>> @@ -94,11 +94,18 @@ void MacOSXAPIChecker::CheckDispatchOnce >>>bool SuggestStatic = false; >>>os << "Call to '" << FName << "' uses"; >>>if (const VarRegion *VR = dyn_cast(RB)) { >>> +const VarDecl *VD = VR->getDecl(); >>> +// FIXME: These should have correct memory space and thus should be >>> filtered >>> +// out earlier. This branch only fires when we're looking from a >>> block, >>> +// which we analyze as a top-level declaration, onto a static local >>> +// in a function that contains the block. >>> +if (VD->isStaticLocal()) >>> + return; >>> // We filtered out globals earlier, so it must be a local variable >>> // or a block variable which is under UnknownSpaceRegion. >>> if (VR != R) >>>os << " memory within"; >>> -if (VR->getDecl()->hasAttr()) >>> +if (VD->hasAttr()) >>>os << " the block variable '"; >>> else >>>os << " the local variable '"; >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Jan 25 04:21:45 >>> 2017 >>> @@ -816,9 +816,11 @@ const VarRegion* MemRegionManager::getVa >>>const StackFrameContext *STC = V.get(); >>> -if (!STC) >>> +if (!STC) { >>> + // FIXME: Assign a more sensible memory space to static locals >>> + // we see from within blocks that we analyze as top-level >>> declarations. >>>sReg = getUnknownRegion(); >>> -else { >>> +} else { >>>if (D->hasLocalStorage()) { >>> sReg = isa(D) || isa(D) >>> ? static_cast>> MemRegion*>(getStackArgumentsRegion(STC)) >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Jan 25 04:21:45 >>> 2017 >>> @@ -1849,6 +1849,8 @@ SVal RegionStoreManager::getBindingForVa >>>// Function-scoped static variables are default-initialized to 0; >>> if they >>> // have an initializer, it would have been processed by now. >>> +// FIXME: This is only true when we're starting analysis from main(). >>> +// We're losing a lot of coverage here. >>> if (isa(MS)) >>>return svalBuilder.makeZeroVal(T); >>> >>> Modified: cfe/trunk/test/Analysis/dispatch-once.m >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dispatch-once.m?rev=293043&r1=293042&r2=293043&view=diff >>> >>> == >>> --- cf
Re: r293043 - [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested blocks.
r294050. Thanks, Hans On Fri, Feb 3, 2017 at 1:50 PM, Anna Zaks wrote: > Fine with merging. Thank you! > Anna. >> On Feb 1, 2017, at 11:00 AM, Hans Wennborg wrote: >> >> If Anna is Ok with it, I'm fine with merging. >> >> Thanks, >> Hans >> >> On Wed, Feb 1, 2017 at 10:29 AM, Artem Dergachev wrote: >>> Hans, >>> >>> This is a fixed and tested version of the previously-merged-and-reverted >>> r292800, do we still have time to land this into 4.0? >>> >>> Thanks, >>> Artem. >>> >>> >>> On 1/25/17 1:21 PM, Artem Dergachev via cfe-commits wrote: Author: dergachev Date: Wed Jan 25 04:21:45 2017 New Revision: 293043 URL: http://llvm.org/viewvc/llvm-project?rev=293043&view=rev Log: [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested blocks. This is an attempt to avoid new false positives caused by the reverted r292800, however the scope of the fix is significantly reduced - some variables are still in incorrect memory spaces. Relevant test cases added. rdar://problem/30105546 rdar://problem/30156693 Differential revision: https://reviews.llvm.org/D28946 Added: cfe/trunk/test/Analysis/null-deref-static.m Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp cfe/trunk/test/Analysis/dispatch-once.m Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp?rev=293043&r1=293042&r2=293043&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp Wed Jan 25 04:21:45 2017 @@ -94,11 +94,18 @@ void MacOSXAPIChecker::CheckDispatchOnce bool SuggestStatic = false; os << "Call to '" << FName << "' uses"; if (const VarRegion *VR = dyn_cast(RB)) { +const VarDecl *VD = VR->getDecl(); +// FIXME: These should have correct memory space and thus should be filtered +// out earlier. This branch only fires when we're looking from a block, +// which we analyze as a top-level declaration, onto a static local +// in a function that contains the block. +if (VD->isStaticLocal()) + return; // We filtered out globals earlier, so it must be a local variable // or a block variable which is under UnknownSpaceRegion. if (VR != R) os << " memory within"; -if (VR->getDecl()->hasAttr()) +if (VD->hasAttr()) os << " the block variable '"; else os << " the local variable '"; Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=293043&r1=293042&r2=293043&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Jan 25 04:21:45 2017 @@ -816,9 +816,11 @@ const VarRegion* MemRegionManager::getVa const StackFrameContext *STC = V.get(); -if (!STC) +if (!STC) { + // FIXME: Assign a more sensible memory space to static locals + // we see from within blocks that we analyze as top-level declarations. sReg = getUnknownRegion(); -else { +} else { if (D->hasLocalStorage()) { sReg = isa(D) || isa(D) ? static_cast>>> MemRegion*>(getStackArgumentsRegion(STC)) Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=293043&r1=293042&r2=293043&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Jan 25 04:21:45 2017 @@ -1849,6 +1849,8 @@ SVal RegionStoreManager::getBindingForVa // Function-scoped static variables are default-initialized to 0; if they // have an initializer, it would have been processed by now. +// FIXME: This is only true when we're starting analysis from main(). +// We're losing a lot of coverage here. if (isa(MS)) return svalBuilder.makeZeroVal(T); Modified: cfe/trunk/test/Analysis/dispatch-once.m URL: http://llvm.org/viewvc/llvm-project
[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
vsk marked an inline comment as done. vsk added a comment. In https://reviews.llvm.org/D29369#665878, @filcab wrote: > Why the switch to `if` instead of a fully-covered switch/case? It lets me avoid repeating two function calls: switch (CGF.getLangOpts().getSignedOverflowBehavior()) { case LangOptions::SOB_Defined: return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul"); case LangOptions::SOB_Undefined: if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) && !CanElideOverflowCheck(CGF.getContext(), Ops)) return EmitOverflowCheckedBinOp(Ops); return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul"); case LangOptions::SOB_Trapping: if (CanElideOverflowCheck(CGF.getContext(), Ops)) return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul"); return EmitOverflowCheckedBinOp(Ops); } Comment at: test/CodeGen/ubsan-promoted-arith.cpp:56 + +// Note: -SHRT_MIN * -SHRT_MIN can overflow. +// filcab wrote: > Nit: Maybe `USHRT_MAX * USHRT_MAX` is more understandable? Yes, I'll fix this. Comment at: test/CodeGen/ubsan-promoted-arith.cpp:99 +// CHECK-LABEL: define signext i8 @_Z4rem3 +// rdar30301609: ubsan_handle_divrem_overflow +char rem3(int i, char c) { return i % c; } filcab wrote: > Maybe put the rdar ID next to the FIXME? Like this it looks like you might > have written that instead of `CHECK` by mistake. I placed the rdar ID here to communicate that the line should become a CHECK line once the bug is fixed. It will go away with D29437. https://reviews.llvm.org/D29369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
vsk marked an inline comment as done. vsk added a comment. In https://reviews.llvm.org/D29369#666308, @vsk wrote: > In https://reviews.llvm.org/D29369#665878, @filcab wrote: > > > Why the switch to `if` instead of a fully-covered switch/case? > > > It lets me avoid repeating two function calls: Ah, sorry about that, I think I understand what you had in mind now. I'll fix that too. https://reviews.llvm.org/D29369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16135: Macro Debug Info support in Clang
aaboud updated this revision to Diff 87020. aaboud marked 4 inline comments as done. aaboud added a comment. Addressed Adrian's comments. Please, let me know if I missed any. Also, I improved the code and added explanation about the file inclusion structure that is being handled in the macro callbacks. https://reviews.llvm.org/D16135 Files: include/clang/CodeGen/ModuleBuilder.h lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h lib/CodeGen/CMakeLists.txt lib/CodeGen/CodeGenAction.cpp lib/CodeGen/MacroPPCallbacks.cpp lib/CodeGen/MacroPPCallbacks.h lib/CodeGen/ModuleBuilder.cpp test/CodeGen/debug-info-global-constant.c Index: lib/CodeGen/ModuleBuilder.cpp === --- lib/CodeGen/ModuleBuilder.cpp +++ lib/CodeGen/ModuleBuilder.cpp @@ -92,6 +92,10 @@ return M.get(); } +CGDebugInfo *getCGDebugInfo() { + return Builder->getModuleDebugInfo(); +} + llvm::Module *ReleaseModule() { return M.release(); } @@ -299,6 +303,10 @@ return static_cast(this)->ReleaseModule(); } +CGDebugInfo *CodeGenerator::getCGDebugInfo() { + return static_cast(this)->getCGDebugInfo(); +} + const Decl *CodeGenerator::GetDeclForMangledName(llvm::StringRef name) { return static_cast(this)->GetDeclForMangledName(name); } Index: lib/CodeGen/MacroPPCallbacks.h === --- lib/CodeGen/MacroPPCallbacks.h +++ lib/CodeGen/MacroPPCallbacks.h @@ -0,0 +1,118 @@ +//===--- MacroPPCallbacks.h -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// This file defines implementation for the macro preprocessors callbacks. +// +//===--===// + +#include "clang/Lex/PPCallbacks.h" + +namespace llvm { +class DIMacroFile; +class DIMacroNode; +} +namespace clang { +class Preprocessor; +class MacroInfo; +class CodeGenerator; + +class MacroPPCallbacks : public PPCallbacks { + /// A pointer to code generator, where debug info generator can be found. + CodeGenerator *Gen; + + /// Preprocessor. + Preprocessor &PP; + + /// Location of recent included file, used for line number. + SourceLocation LastHashLoc; + + /// Counts current number of command line included files, which was entered + /// and was not exited yet. + int CommandIncludeFiles = 0; + + enum FileScopeStatus { +NoScope = 0, // Scope is not initialized yet. +InitializedScope, // Main file scope is initialized but not set yet. +BuiltinScope, // and file scopes. +CommandLineScopeIncludes, // Included file, from file, scope. +MainFileScope // Main file scope. + }; + FileScopeStatus Status; + + /// Parent contains all entered files that were not exited yet according to + /// the inclusion order. + llvm::SmallVector Scopes; + + /// Get current DIMacroFile scope. + /// \return current DIMacroFile scope or nullptr if there is no such scope. + llvm::DIMacroFile *getCurrentScope(); + + /// Get current line location or invalid location. + /// \param Loc current line location. + /// \return current line location \p `Loc`, or invalid location if it's in a + /// skipped file scope. + SourceLocation getCorrectLocation(SourceLocation Loc); + + /// Use the passed preprocessor to write the macro name and value from the + /// given macro info and identifier info into the given \p `Name` and \p + /// `Value` output streams. + /// + /// \param II Identifier info, used to get the Macro name. + /// \param MI Macro info, used to get the Macro argumets and values. + /// \param PP Preprocessor. + /// \param [out] Name Place holder for returned macro name and arguments. + /// \param [out] Value Place holder for returned macro value. + static void writeMacroDefinition(const IdentifierInfo &II, + const MacroInfo &MI, Preprocessor &PP, + raw_ostream &Name, raw_ostream &Value); + + /// Update current file scope status to next file scope. + void updateStatusToNextScope(); + + /// Handle the case when entering a file. + /// + /// \param Loc Indicates the new location. + /// \Return true if file scope status should be updated. + void FileEntered(SourceLocation Loc); + + /// Handle the case when exiting a file. + /// + /// \Return true if file scope status should be updated. + void FileExited(SourceLocation Loc); + +public: + MacroPPCallbacks(CodeGenerator *Gen, Preprocessor &PP); + + /// Callback invoked whenever a source file is entered or exited. + /// + /// \param Loc Indicates the new location. + /// \param PrevFID t
[PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin
MatzeB updated this revision to Diff 87023. Repository: rL LLVM https://reviews.llvm.org/D29479 Files: lib/Driver/Tools.cpp test/Driver/darwin-ld-pthread.c Index: test/Driver/darwin-ld-pthread.c === --- /dev/null +++ test/Driver/darwin-ld-pthread.c @@ -0,0 +1,4 @@ +// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s + +// There is nothing to do at link time to get pthread support. But do not warn. +// CHECK-NOT: argument unused during compilation: '-pthread' Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -8718,6 +8718,10 @@ } } + // No need to do anything for pthreads. Claim argument to avoid warning. + Args.ClaimAllArgs(options::OPT_pthread); + Args.ClaimAllArgs(options::OPT_pthreads); + const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath()); std::unique_ptr Cmd = llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs); Index: test/Driver/darwin-ld-pthread.c === --- /dev/null +++ test/Driver/darwin-ld-pthread.c @@ -0,0 +1,4 @@ +// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s + +// There is nothing to do at link time to get pthread support. But do not warn. +// CHECK-NOT: argument unused during compilation: '-pthread' Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -8718,6 +8718,10 @@ } } + // No need to do anything for pthreads. Claim argument to avoid warning. + Args.ClaimAllArgs(options::OPT_pthread); + Args.ClaimAllArgs(options::OPT_pthreads); + const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath()); std::unique_ptr Cmd = llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r294058 - [x86] fix tests with wrong dependency to pass because they broke with r294049
Author: spatel Date: Fri Feb 3 16:03:47 2017 New Revision: 294058 URL: http://llvm.org/viewvc/llvm-project?rev=294058&view=rev Log: [x86] fix tests with wrong dependency to pass because they broke with r294049 Modified: cfe/trunk/test/CodeGen/avx512-reduceMinMaxIntrin.c Modified: cfe/trunk/test/CodeGen/avx512-reduceMinMaxIntrin.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512-reduceMinMaxIntrin.c?rev=294058&r1=294057&r2=294058&view=diff == --- cfe/trunk/test/CodeGen/avx512-reduceMinMaxIntrin.c (original) +++ cfe/trunk/test/CodeGen/avx512-reduceMinMaxIntrin.c Fri Feb 3 16:03:47 2017 @@ -1,3 +1,5 @@ +// FIXME: We should not be testing with -O2 (ie, a dependency on the entire IR optimizer). + // RUN: %clang_cc1 -ffreestanding %s -O2 -triple=x86_64-apple-darwin -target-cpu skylake-avx512 -emit-llvm -o - -Wall -Werror |opt -instnamer -S |FileCheck %s #include @@ -202,7 +204,7 @@ double test_mm512_mask_reduce_min_pd(__m int test_mm512_reduce_max_epi32(__m512i __W){ // CHECK: %tmp = bitcast <8 x i64> %__W to <16 x i32> // CHECK: %shuffle1.i = shufflevector <16 x i32> %tmp, <16 x i32> undef, <16 x i32> - // CHECK: %tmp1 = icmp sgt <16 x i32> %tmp, %shuffle1.i + // CHECK: %tmp1 = icmp slt <16 x i32> %shuffle1.i, %tmp // CHECK: %tmp2 = select <16 x i1> %tmp1, <16 x i32> %tmp, <16 x i32> %shuffle1.i // CHECK: %shuffle3.i = shufflevector <16 x i32> %tmp2, <16 x i32> undef, <16 x i32> // CHECK: %tmp3 = icmp sgt <16 x i32> %tmp2, %shuffle3.i @@ -223,7 +225,7 @@ int test_mm512_reduce_max_epi32(__m512i unsigned int test_mm512_reduce_max_epu32(__m512i __W){ // CHECK: %tmp = bitcast <8 x i64> %__W to <16 x i32> // CHECK: %shuffle1.i = shufflevector <16 x i32> %tmp, <16 x i32> undef, <16 x i32> - // CHECK: %tmp1 = icmp ugt <16 x i32> %tmp, %shuffle1.i + // CHECK: %tmp1 = icmp ult <16 x i32> %shuffle1.i, %tmp // CHECK: %tmp2 = select <16 x i1> %tmp1, <16 x i32> %tmp, <16 x i32> %shuffle1.i // CHECK: %shuffle3.i = shufflevector <16 x i32> %tmp2, <16 x i32> undef, <16 x i32> // CHECK: %tmp3 = icmp ugt <16 x i32> %tmp2, %shuffle3.i @@ -258,7 +260,7 @@ float test_mm512_reduce_max_ps(__m512 __ int test_mm512_reduce_min_epi32(__m512i __W){ // CHECK: %tmp = bitcast <8 x i64> %__W to <16 x i32> // CHECK: %shuffle1.i = shufflevector <16 x i32> %tmp, <16 x i32> undef, <16 x i32> - // CHECK: %tmp1 = icmp slt <16 x i32> %tmp, %shuffle1.i + // CHECK: %tmp1 = icmp sgt <16 x i32> %shuffle1.i, %tmp // CHECK: %tmp2 = select <16 x i1> %tmp1, <16 x i32> %tmp, <16 x i32> %shuffle1.i // CHECK: %shuffle3.i = shufflevector <16 x i32> %tmp2, <16 x i32> undef, <16 x i32> // CHECK: %tmp3 = icmp slt <16 x i32> %tmp2, %shuffle3.i @@ -279,7 +281,7 @@ int test_mm512_reduce_min_epi32(__m512i unsigned int test_mm512_reduce_min_epu32(__m512i __W){ // CHECK: %tmp = bitcast <8 x i64> %__W to <16 x i32> // CHECK: %shuffle1.i = shufflevector <16 x i32> %tmp, <16 x i32> undef, <16 x i32> - // CHECK: %tmp1 = icmp ult <16 x i32> %tmp, %shuffle1.i + // CHECK: %tmp1 = icmp ugt <16 x i32> %shuffle1.i, %tmp // CHECK: %tmp2 = select <16 x i1> %tmp1, <16 x i32> %tmp, <16 x i32> %shuffle1.i // CHECK: %shuffle3.i = shufflevector <16 x i32> %tmp2, <16 x i32> undef, <16 x i32> // CHECK: %tmp3 = icmp ult <16 x i32> %tmp2, %shuffle3.i ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin
MatzeB updated this revision to Diff 87024. MatzeB added a comment. Address review comments: - Simplify test - Only perform the ClaimAll() if we actually link libc, so we get the warning back when combining -nostdlib/-nodefaultlibs with -pthread Repository: rL LLVM https://reviews.llvm.org/D29479 Files: lib/Driver/Tools.cpp test/Driver/darwin-ld-pthread.c Index: test/Driver/darwin-ld-pthread.c === --- /dev/null +++ test/Driver/darwin-ld-pthread.c @@ -0,0 +1,4 @@ +// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s + +// There is nothing to do at link time to get pthread support. But do not warn. +// CHECK-NOT: argument unused during compilation: '-pthread' Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -8696,6 +8696,10 @@ // Let the tool chain choose which runtime library to link. getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs); + +// No need to do anything for pthreads. Claim argument to avoid warning. +Args.ClaimAllArgs(options::OPT_pthread); +Args.ClaimAllArgs(options::OPT_pthreads); } if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) { Index: test/Driver/darwin-ld-pthread.c === --- /dev/null +++ test/Driver/darwin-ld-pthread.c @@ -0,0 +1,4 @@ +// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s + +// There is nothing to do at link time to get pthread support. But do not warn. +// CHECK-NOT: argument unused during compilation: '-pthread' Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -8696,6 +8696,10 @@ // Let the tool chain choose which runtime library to link. getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs); + +// No need to do anything for pthreads. Claim argument to avoid warning. +Args.ClaimAllArgs(options::OPT_pthread); +Args.ClaimAllArgs(options::OPT_pthreads); } if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin
dim added a comment. In https://reviews.llvm.org/D28213#666269, @dim wrote: > In https://reviews.llvm.org/D28213#666196, @mgorny wrote: > > > > ... >> What's the value of `__atomic_always_lock_free(sizeof(long long), 0)` for >> gcc and clang? > > For gcc, it is always 0, for clang (I tested 3.4.1 through 4.0.0) it is > always 1. Maybe that was always incorrect on 32-bit FreeBSD, then? Hmm, I just noticed the following rather disappointing comment in `tools/clang/lib/Basic/Targets.cpp`: X86_32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : X86TargetInfo(Triple, Opts) { [...] // x86-32 has atomics up to 8 bytes // FIXME: Check that we actually have cmpxchg8b before setting // MaxAtomicInlineWidth. (cmpxchg8b is an i586 instruction.) MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64; So this can never have worked properly for e.g. i486 and i586... Repository: rL LLVM https://reviews.llvm.org/D28213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r294008 - [Sema][ObjC++] Typo correction should handle ivars and properties
It looks like the only cases it should have any real impact on are those where we would previously miscompile/crash, so this seems OK to me to merge to Clang 4. On 3 February 2017 at 13:45, Hans Wennborg wrote: > IIUC, this isn't strictly fixing a regression from 3.9, but it looks > like a pretty small diff. > > Richard, what do you think? > > On Fri, Feb 3, 2017 at 6:37 AM, Alex L wrote: > > Hi Hans, > > > > Is there any chance we can merge this for 4.0? It fixed a nasty bug where > > clang didn't catch invalid ObjC++ code during semantic analysis which > led to > > invalid object files or crashes in CodeGen. > > > > Cheers, > > Alex > > > > On 3 February 2017 at 14:22, Alex Lorenz via cfe-commits > > wrote: > >> > >> Author: arphaman > >> Date: Fri Feb 3 08:22:33 2017 > >> New Revision: 294008 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=294008&view=rev > >> Log: > >> [Sema][ObjC++] Typo correction should handle ivars and properties > >> > >> After r260016 and r260017 disabled typo correction for ivars and > >> properties > >> clang didn't report errors about unresolved identifier in the base of > ivar > >> and > >> property ref expressions. This meant that clang invoked CodeGen on > invalid > >> AST > >> which then caused a crash. > >> > >> This commit re-enables typo correction for ivars and properites, and > fixes > >> the > >> PR25113 & PR26486 (that were originally fixed in r260017 and r260016) > in a > >> different manner by transforming the Objective-C ivar reference > expression > >> with > >> 'IsFreeIvar' preserved. > >> > >> rdar://30310772 > >> > >> Modified: > >> cfe/trunk/lib/Sema/SemaExprCXX.cpp > >> cfe/trunk/lib/Sema/TreeTransform.h > >> cfe/trunk/test/SemaObjCXX/typo-correction.mm > >> > >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaExprCXX.cpp?rev=294008&r1=294007&r2=294008&view=diff > >> > >> > == > >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) > >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb 3 08:22:33 2017 > >> @@ -7250,14 +7250,6 @@ public: > >> > >>ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); } > >> > >> - ExprResult TransformObjCPropertyRefExpr(ObjCPropertyRefExpr *E) { > >> -return Owned(E); > >> - } > >> - > >> - ExprResult TransformObjCIvarRefExpr(ObjCIvarRefExpr *E) { > >> -return Owned(E); > >> - } > >> - > >>ExprResult Transform(Expr *E) { > >> ExprResult Res; > >> while (true) { > >> > >> Modified: cfe/trunk/lib/Sema/TreeTransform.h > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > TreeTransform.h?rev=294008&r1=294007&r2=294008&view=diff > >> > >> > == > >> --- cfe/trunk/lib/Sema/TreeTransform.h (original) > >> +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Feb 3 08:22:33 2017 > >> @@ -2982,16 +2982,17 @@ public: > >>ExprResult RebuildObjCIvarRefExpr(Expr *BaseArg, ObjCIvarDecl *Ivar, > >>SourceLocation IvarLoc, > >>bool IsArrow, bool > IsFreeIvar) > >> { > >> -// FIXME: We lose track of the IsFreeIvar bit. > >> CXXScopeSpec SS; > >> DeclarationNameInfo NameInfo(Ivar->getDeclName(), IvarLoc); > >> -return getSema().BuildMemberReferenceExpr(BaseArg, > >> BaseArg->getType(), > >> - /*FIXME:*/IvarLoc, > IsArrow, > >> - SS, SourceLocation(), > >> - > >> /*FirstQualifierInScope=*/nullptr, > >> - NameInfo, > >> - /*TemplateArgs=*/nullptr, > >> - /*S=*/nullptr); > >> +ExprResult Result = getSema().BuildMemberReferenceExpr( > >> +BaseArg, BaseArg->getType(), > >> +/*FIXME:*/ IvarLoc, IsArrow, SS, SourceLocation(), > >> +/*FirstQualifierInScope=*/nullptr, NameInfo, > >> +/*TemplateArgs=*/nullptr, > >> +/*S=*/nullptr); > >> +if (IsFreeIvar && Result.isUsable()) > >> + cast(Result.get())->setIsFreeIvar(IsFreeIvar); > >> +return Result; > >>} > >> > >>/// \brief Build a new Objective-C property reference expression. > >> > >> Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > SemaObjCXX/typo-correction.mm?rev=294008&r1=294007&r2=294008&view=diff > >> > >> > == > >> --- cfe/trunk/test/SemaObjCXX/typo-correction.mm (original) > >> +++ cfe/trunk/test/SemaObjCXX/typo-correction.mm Fri Feb 3 08:22:33 > 2017 > >> @@ -21,3 +21,18 @@ public: > >>self.m_prop2 = new ClassB(m_prop1); // expected-error {{use of > >> und
[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
vsk updated this revision to Diff 87030. vsk edited the summary of this revision. vsk added a comment. - Use switches per Filipe's comment, and fix a comment in the test case. https://reviews.llvm.org/D29369 Files: lib/CodeGen/CGExprScalar.cpp test/CodeGen/compound-assign-overflow.c test/CodeGen/ubsan-promoted-arith.cpp test/CodeGen/unsigned-promotion.c Index: test/CodeGen/unsigned-promotion.c === --- test/CodeGen/unsigned-promotion.c +++ test/CodeGen/unsigned-promotion.c @@ -7,53 +7,6 @@ // RUN: -fsanitize=unsigned-integer-overflow | FileCheck %s --check-prefix=CHECKU unsigned short si, sj, sk; -unsigned char ci, cj, ck; - -extern void opaqueshort(unsigned short); -extern void opaquechar(unsigned char); - -// CHECKS-LABEL: define void @testshortadd() -// CHECKU-LABEL: define void @testshortadd() -void testshortadd() { - // CHECKS:load i16, i16* @sj - // CHECKS:load i16, i16* @sk - // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS:call void @__ubsan_handle_add_overflow - // - // CHECKU: [[T1:%.*]] = load i16, i16* @sj - // CHECKU: [[T2:%.*]] = zext i16 [[T1]] - // CHECKU: [[T3:%.*]] = load i16, i16* @sk - // CHECKU: [[T4:%.*]] = zext i16 [[T3]] - // CHECKU-NOT: llvm.sadd - // CHECKU-NOT: llvm.uadd - // CHECKU: [[T5:%.*]] = add nsw i32 [[T2]], [[T4]] - - si = sj + sk; -} - -// CHECKS-LABEL: define void @testshortsub() -// CHECKU-LABEL: define void @testshortsub() -void testshortsub() { - - // CHECKS:load i16, i16* @sj - // CHECKS:load i16, i16* @sk - // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS:call void @__ubsan_handle_sub_overflow - // - // CHECKU: [[T1:%.*]] = load i16, i16* @sj - // CHECKU: [[T2:%.*]] = zext i16 [[T1]] - // CHECKU: [[T3:%.*]] = load i16, i16* @sk - // CHECKU: [[T4:%.*]] = zext i16 [[T3]] - // CHECKU-NOT: llvm.ssub - // CHECKU-NOT: llvm.usub - // CHECKU: [[T5:%.*]] = sub nsw i32 [[T2]], [[T4]] - - si = sj - sk; -} // CHECKS-LABEL: define void @testshortmul() // CHECKU-LABEL: define void @testshortmul() @@ -75,69 +28,3 @@ // CHECKU: [[T5:%.*]] = mul nsw i32 [[T2]], [[T4]] si = sj * sk; } - -// CHECKS-LABEL: define void @testcharadd() -// CHECKU-LABEL: define void @testcharadd() -void testcharadd() { - - // CHECKS:load i8, i8* @cj - // CHECKS:load i8, i8* @ck - // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS:call void @__ubsan_handle_add_overflow - // - // CHECKU: [[T1:%.*]] = load i8, i8* @cj - // CHECKU: [[T2:%.*]] = zext i8 [[T1]] - // CHECKU: [[T3:%.*]] = load i8, i8* @ck - // CHECKU: [[T4:%.*]] = zext i8 [[T3]] - // CHECKU-NOT: llvm.sadd - // CHECKU-NOT: llvm.uadd - // CHECKU: [[T5:%.*]] = add nsw i32 [[T2]], [[T4]] - - ci = cj + ck; -} - -// CHECKS-LABEL: define void @testcharsub() -// CHECKU-LABEL: define void @testcharsub() -void testcharsub() { - - // CHECKS:load i8, i8* @cj - // CHECKS:load i8, i8* @ck - // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS:call void @__ubsan_handle_sub_overflow - // - // CHECKU: [[T1:%.*]] = load i8, i8* @cj - // CHECKU: [[T2:%.*]] = zext i8 [[T1]] - // CHECKU: [[T3:%.*]] = load i8, i8* @ck - // CHECKU: [[T4:%.*]] = zext i8 [[T3]] - // CHECKU-NOT: llvm.ssub - // CHECKU-NOT: llvm.usub - // CHECKU: [[T5:%.*]] = sub nsw i32 [[T2]], [[T4]] - - ci = cj - ck; -} - -// CHECKS-LABEL: define void @testcharmul() -// CHECKU-LABEL: define void @testcharmul() -void testcharmul() { - - // CHECKS:load i8, i8* @cj - // CHECKS:load i8, i8* @ck - // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.smul.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS:call void @__ubsan_handle_mul_overflow - // - // CHECKU: [[T1:%.*]] = load i8, i8* @cj - // CHECKU: [[T2:%.*]] = zext i8 [[T1]] - // CHECKU:
Re: r294008 - [Sema][ObjC++] Typo correction should handle ivars and properties
Thanks! r294059. On Fri, Feb 3, 2017 at 2:29 PM, Richard Smith wrote: > It looks like the only cases it should have any real impact on are those > where we would previously miscompile/crash, so this seems OK to me to merge > to Clang 4. > > > On 3 February 2017 at 13:45, Hans Wennborg wrote: >> >> IIUC, this isn't strictly fixing a regression from 3.9, but it looks >> like a pretty small diff. >> >> Richard, what do you think? >> >> On Fri, Feb 3, 2017 at 6:37 AM, Alex L wrote: >> > Hi Hans, >> > >> > Is there any chance we can merge this for 4.0? It fixed a nasty bug >> > where >> > clang didn't catch invalid ObjC++ code during semantic analysis which >> > led to >> > invalid object files or crashes in CodeGen. >> > >> > Cheers, >> > Alex >> > >> > On 3 February 2017 at 14:22, Alex Lorenz via cfe-commits >> > wrote: >> >> >> >> Author: arphaman >> >> Date: Fri Feb 3 08:22:33 2017 >> >> New Revision: 294008 >> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=294008&view=rev >> >> Log: >> >> [Sema][ObjC++] Typo correction should handle ivars and properties >> >> >> >> After r260016 and r260017 disabled typo correction for ivars and >> >> properties >> >> clang didn't report errors about unresolved identifier in the base of >> >> ivar >> >> and >> >> property ref expressions. This meant that clang invoked CodeGen on >> >> invalid >> >> AST >> >> which then caused a crash. >> >> >> >> This commit re-enables typo correction for ivars and properites, and >> >> fixes >> >> the >> >> PR25113 & PR26486 (that were originally fixed in r260017 and r260016) >> >> in a >> >> different manner by transforming the Objective-C ivar reference >> >> expression >> >> with >> >> 'IsFreeIvar' preserved. >> >> >> >> rdar://30310772 >> >> >> >> Modified: >> >> cfe/trunk/lib/Sema/SemaExprCXX.cpp >> >> cfe/trunk/lib/Sema/TreeTransform.h >> >> cfe/trunk/test/SemaObjCXX/typo-correction.mm >> >> >> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp >> >> URL: >> >> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=294008&r1=294007&r2=294008&view=diff >> >> >> >> >> >> == >> >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) >> >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb 3 08:22:33 2017 >> >> @@ -7250,14 +7250,6 @@ public: >> >> >> >>ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); } >> >> >> >> - ExprResult TransformObjCPropertyRefExpr(ObjCPropertyRefExpr *E) { >> >> -return Owned(E); >> >> - } >> >> - >> >> - ExprResult TransformObjCIvarRefExpr(ObjCIvarRefExpr *E) { >> >> -return Owned(E); >> >> - } >> >> - >> >>ExprResult Transform(Expr *E) { >> >> ExprResult Res; >> >> while (true) { >> >> >> >> Modified: cfe/trunk/lib/Sema/TreeTransform.h >> >> URL: >> >> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=294008&r1=294007&r2=294008&view=diff >> >> >> >> >> >> == >> >> --- cfe/trunk/lib/Sema/TreeTransform.h (original) >> >> +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Feb 3 08:22:33 2017 >> >> @@ -2982,16 +2982,17 @@ public: >> >>ExprResult RebuildObjCIvarRefExpr(Expr *BaseArg, ObjCIvarDecl *Ivar, >> >>SourceLocation IvarLoc, >> >>bool IsArrow, bool >> >> IsFreeIvar) >> >> { >> >> -// FIXME: We lose track of the IsFreeIvar bit. >> >> CXXScopeSpec SS; >> >> DeclarationNameInfo NameInfo(Ivar->getDeclName(), IvarLoc); >> >> -return getSema().BuildMemberReferenceExpr(BaseArg, >> >> BaseArg->getType(), >> >> - /*FIXME:*/IvarLoc, >> >> IsArrow, >> >> - SS, SourceLocation(), >> >> - >> >> /*FirstQualifierInScope=*/nullptr, >> >> - NameInfo, >> >> - >> >> /*TemplateArgs=*/nullptr, >> >> - /*S=*/nullptr); >> >> +ExprResult Result = getSema().BuildMemberReferenceExpr( >> >> +BaseArg, BaseArg->getType(), >> >> +/*FIXME:*/ IvarLoc, IsArrow, SS, SourceLocation(), >> >> +/*FirstQualifierInScope=*/nullptr, NameInfo, >> >> +/*TemplateArgs=*/nullptr, >> >> +/*S=*/nullptr); >> >> +if (IsFreeIvar && Result.isUsable()) >> >> + cast(Result.get())->setIsFreeIvar(IsFreeIvar); >> >> +return Result; >> >>} >> >> >> >>/// \brief Build a new Objective-C property reference expression. >> >> >> >> Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm >> >> URL: >> >> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/typo-correction.mm?rev=294008&r1=294007&r2=294008&view=diff >> >> >> >> >> >> == >> >> --- cfe/trunk/test/SemaObjCXX/typo-cor
[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type
ahatanak updated this revision to Diff 87016. ahatanak marked 2 inline comments as done. ahatanak added a comment. Turning the warning on by default caused clang to issue warnings in many other cases, including Objective-C methods returning nil, which was something r240153 tried to avoid. If we want to disable the warning in Sema::ImpCastExprToType when the cast is part of a return statement of an ObjC method, we'll probably have to pass down a flag or something, which is probably not a good idea. Instead of trying to issue the warning in Sema::ImpCastExprToType, the new patch checks whether there is a null constant in Sema::AddInitializerToDecl (which is needed to issue the first warning in null_constant_to_nonnull.c) and computeConditionalNullability (which is needed for the second warning). Also, it adds method Sema::checkNonNullExpr so that CheckNonNullExpr can be called from files other than SemaChecking.cpp. https://reviews.llvm.org/D22391 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp test/Analysis/nullability-no-arc.mm test/Analysis/nullability_nullonly.mm test/Sema/conditional-expr.c test/Sema/null_constant_to_nonnull.c Index: test/Sema/null_constant_to_nonnull.c === --- /dev/null +++ test/Sema/null_constant_to_nonnull.c @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -Wnullable-to-nonnull-conversion %s -verify + +void null_const_to_nonnull(int c) { + int * _Nonnull p0 = 0; // expected-warning{{implicitly casting a null constant to non-nullable pointer type 'int * _Nonnull'}} + int * _Nonnull p1; + int * _Nonnull p2 = c ? p1 : 0; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}} +} Index: test/Sema/conditional-expr.c === --- test/Sema/conditional-expr.c +++ test/Sema/conditional-expr.c @@ -17,7 +17,7 @@ dp = ip; // expected-warning {{incompatible pointer types assigning to 'double *' from 'int *'}} dp = 0 ? (double *)0 : (void *)0; vp = 0 ? (double *)0 : (void *)0; - ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer types assigning to 'int *' from 'double *'}} + ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer types assigning to 'int *' from 'double * _Nullable'}} const int *cip; vp = (0 ? vp : cip); // expected-warning {{discards qualifiers}} @@ -90,7 +90,7 @@ int f0(int a) { // GCC considers this a warning. - return a ? f1() : nil; // expected-warning {{pointer/integer type mismatch in conditional expression ('int' and 'void *')}} expected-warning {{incompatible pointer to integer conversion returning 'void *' from a function with result type 'int'}} + return a ? f1() : nil; // expected-warning {{pointer/integer type mismatch in conditional expression ('int' and 'void *')}} expected-warning {{incompatible pointer to integer conversion returning 'void * _Nullable' from a function with result type 'int'}} } int f2(int x) { Index: test/Analysis/nullability_nullonly.mm === --- test/Analysis/nullability_nullonly.mm +++ test/Analysis/nullability_nullonly.mm @@ -100,7 +100,7 @@ } void testObjCARCExplicitZeroInitialization() { - TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} + TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} expected-warning{{implicitly casting a null constant to non-nullable pointer type 'TestObject * _Nonnull __strong'}} } // Under ARC, returned expressions of ObjC objects types are are implicitly Index: test/Analysis/nullability-no-arc.mm === --- test/Analysis/nullability-no-arc.mm +++ test/Analysis/nullability-no-arc.mm @@ -43,7 +43,7 @@ } void testObjCNonARCExplicitZeroInitialization() { - TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} + TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} expected-warning {{implicitly casting a null constant to non-nullable pointer type 'TestObject * _Nonnull'}} } @interface ClassWithInitializers : NSObject Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -7097,20 +7097,26 @@ } /// Compute the nullability of a conditional expression. -static QualType com
Re: [PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin
LGTM. > On 2017-Feb-03, at 14:16, Matthias Braun via Phabricator > wrote: > > MatzeB updated this revision to Diff 87024. > MatzeB added a comment. > > Address review comments: > > - Simplify test > - Only perform the ClaimAll() if we actually link libc, so we get the warning > back when combining -nostdlib/-nodefaultlibs with -pthread > > > Repository: > rL LLVM > > https://reviews.llvm.org/D29479 > > Files: > lib/Driver/Tools.cpp > test/Driver/darwin-ld-pthread.c > > > Index: test/Driver/darwin-ld-pthread.c > === > --- /dev/null > +++ test/Driver/darwin-ld-pthread.c > @@ -0,0 +1,4 @@ > +// RUN: %clang -Wunused-command-line-argument -pthread -target > x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s > + > +// There is nothing to do at link time to get pthread support. But do not > warn. > +// CHECK-NOT: argument unused during compilation: '-pthread' > Index: lib/Driver/Tools.cpp > === > --- lib/Driver/Tools.cpp > +++ lib/Driver/Tools.cpp > @@ -8696,6 +8696,10 @@ > > // Let the tool chain choose which runtime library to link. > getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs); > + > +// No need to do anything for pthreads. Claim argument to avoid warning. > +Args.ClaimAllArgs(options::OPT_pthread); > +Args.ClaimAllArgs(options::OPT_pthreads); > } > > if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) { > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r293518 - [ASTMatchers] Sprinkle some constexpr on the global matcher constructors.
Note that this also failed on Linux: http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/2728 This looks like a modules compiler bug, as opposed to a problem in the commit itself. Alex, can you file a PR for this? > On 2017-Feb-01, at 07:35, Alex L via cfe-commits > wrote: > > I've narrowed the problem down to the difference in the linkage type for > "clang::ast_matchers::recordDecl" between the module and non-module build: > > Module: > @_ZN5clang12ast_matchersL10recordDeclE = external global > %"class.clang::ast_matchers::internal::VariadicDynCastAllOfMatcher.847", > align 1 > > Non-module: > @_ZN5clang12ast_matchersL10recordDeclE = internal constant > %"class.clang::ast_matchers::internal::VariadicDynCastAllOfMatcher.916" > undef, align 1, !dbg !7 > > On 1 February 2017 at 11:03, Alex L wrote: > Hi Benjamin, > > This commit has caused a linking in our stage 2 modules buildbot at > http://lab.llvm.org:8080/green/job/clang-stage2-cmake-modulesRDA/. > Specifically, after this commit 'clang-reorder-fields' gets the following > linking error: > > FAILED: bin/clang-reorder-fields > : && > /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-modulesRDA@2/host-compiler/bin/clang++ >-fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter > -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic > -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor > -Wdelete-non-virtual-dtor -Wstring-conversion -Werror=date-time -std=c++11 > -fmodules > -fmodules-cache-path=/Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-modulesRDA@2/clang-build/module.cache > -fcxx-modules -gmodules -fcolor-diagnostics -fno-common -Woverloaded-virtual > -Wno-nested-anon-types -O2 -g -DNDEBUG -Wl,-search_paths_first > -Wl,-headerpad_max_install_names -Wl,-dead_strip > tools/clang/tools/extra/clang-reorder-fields/tool/CMakeFiles/clang-reorder-fields.dir/ClangReorderFields.cpp.o > -o bin/clang-reorder-fields lib/libLLVMSupport.a lib/libclangBasic.a > lib/libclangFrontend.a lib/libclangReorderFields.a lib/libclangRewrite.a > lib/libclangTooling.a lib/libclangToolingCore.a lib/libclangIndex.a > lib/libclangFrontend.a lib/libclangParse.a lib/libLLVMMCParser.a > lib/libclangSerialization.a lib/libclangSema.a lib/libclangEdit.a > lib/libclangAnalysis.a lib/libLLVMBitReader.a lib/libLLVMProfileData.a > lib/libclangDriver.a lib/libLLVMOption.a lib/libclangASTMatchers.a > lib/libclangFormat.a lib/libclangToolingCore.a lib/libclangRewrite.a > lib/libclangAST.a lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a > lib/libLLVMMC.a lib/libLLVMSupport.a -lcurses -lz -lm lib/libLLVMDemangle.a > -Wl,-rpath,@loader_path/../lib && : > Undefined symbols for architecture x86_64: > "clang::ast_matchers::recordDecl", referenced from: > clang::reorder_fields::(anonymous > namespace)::ReorderingConsumer::HandleTranslationUnit(clang::ASTContext&) in > libclangReorderFields.a(ReorderFieldsAction.cpp.o) > ld: symbol(s) not found for architecture x86_64 > > This might be a bug/regression in clang and modules since in my opinion the > link should succeed. I've reverted your commit in r293759 while we are > investigating. > > Thanks, > Alex > > > On 30 January 2017 at 18:20, Benjamin Kramer via cfe-commits > wrote: > Author: d0k > Date: Mon Jan 30 12:20:00 2017 > New Revision: 293518 > > URL: http://llvm.org/viewvc/llvm-project?rev=293518&view=rev > Log: > [ASTMatchers] Sprinkle some constexpr on the global matcher constructors. > > This dramatically reduces the size of the global constructors we emit > for those variables in debug mode. > > Modified: > cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h > > Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=293518&r1=293517&r2=293518&view=diff > == > --- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original) > +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Mon Jan 30 > 12:20:00 2017 > @@ -1459,7 +1459,7 @@ class VariadicDynCastAllOfMatcher > : public VariadicFunction, Matcher, >makeDynCastAllOfComposite> { > public: > - VariadicDynCastAllOfMatcher() {} > + constexpr VariadicDynCastAllOfMatcher() {} > }; > > /// \brief A \c VariadicAllOfMatcher object is a variadic functor that > takes > @@ -1477,7 +1477,7 @@ class VariadicAllOfMatcher > : public VariadicFunction, Matcher, >makeAllOfComposite> { > public: > - VariadicAllOfMatcher() {} > + constexpr VariadicAllOfMatcher() {} > }; > > /// \brief Matches nodes of type \c TLoc for which the inner > @@ -1598,7 +1598,7 @@ public: > >struct Func >: public VariadicFunction, &Self::create> { > -Func() {} > +
[PATCH] D29520: Lit C++11 Compatibility - Microsoft diagnostics
tigerleapgorge created this revision. I am continuing to make Lit tests C++11 compatible. This patch contains 4 tests, previously in review https://reviews.llvm.org/D20710 and https://reviews.llvm.org/D21626. test/SemaCXX/MicrosoftExtensions.cpp This test checks for Microsoft extensions. Portions of this test check for unsupported C++11 features when compiling at C++98. Guard all such diagnostics under C++98. Base destructor being marked with “throw()”, derived destructor is not. This no longer results in the following diagnostics in C++11. C++98: warning: exception specification of overriding function is more lax than base version [-Wmicrosoft-exception-spec] note: overridden virtual function is here Enum with underlying type is now supported in C++11. Guard the following under C++98. C++98: warning: enumeration types with a fixed underlying type are a C++11 extension [-Wc++11-extensions] C++98: warning: enumeration types with a fixed underlying type are a C++11 extension [-Wc++11-extensions] “override” is now supported in C++11. Guard the following under C++98. C++98: warning: 'override' keyword is a C++11 extension [-Wc++11-extensions] test/SemaCXX/implicit-virtual-member-functions.cpp Change in diagnostics (3 instances) C++98: error: no suitable member 'operator delete' in 'B' note: member 'operator delete' declared here note: implicit destructor for 'B' first required here C++11: error: deleted function '~B' cannot override a non-deleted function note: overridden virtual function is here Added diagnostics in C++11 when target is Microsoft. C++11: error: attempt to use a deleted function note: virtual destructor requires an unambiguous, accessible 'operator delete' test/SemaCXX/virtual-base-used.cpp The base class explicitly declares a public virtual destructor. The derived class does not explicitly declare a destructor. The derived class contains a member with an inaccessible private destructor. In C++98, Clang Warns about the private destructor then gives a Note at class instantiation (MSABI) or class method definition. In C++11, The derived class having a member that can not be destroyed means the derived class’s own implicit destructor is deleted. Therefore, Clang issues an Error on the derived class’s deleted destructor trying to overwrite base class’s non-deleted destructor. Furthermore, Clang also issues Errors on classes further down the inheritance chain with explicitly declared destructors trying to overwrite first derived class’s implicitly deleted destructor. This test is subdivided into three sections. Each section has its own inheritance chain. Section 1: A is the base struct with a virtual destructor. B derives from A. B has a member class instance ‘x’ with an inaccessible destructor. D derives from B. D has an explicitly declared destructor. In C++98, Clang issues an Error about B’s x having no accessible destructor. In C++11, Clang issues 2 Errors. First Error on B’s implicitly deleted destructor inheriting A’s explicitly declared destructor. Second Error on D’s explicitly declared destructor inheriting B’s implicitly deleted destructor. C++98: error: field of type 'NoDestroy' has private destructor note: implicitly declared private here note: implicit destructor for 'B' first required here C++11: error: deleted function '~B' cannot override a non-deleted function note: overridden virtual function is here error: non-deleted function '~D' cannot override a deleted function note: overridden virtual function is here Section 2: A is the base struct with a virtual destructor. E derives A. E also has a member class instance x with no destructor. F derives from E and has no explicitly declared destructor. G derives from F and has an explicitly declared destructor. In C++98, Clang issues an Error about E’s x having no accessible destructor. In C++11, Clang issues 3 Errors. First Error about E’s implicitly deleted destructor inheriting A’s explicitly declared destructor. Second Error about F’s implicitly declared destructor inheriting E’s implicitly deleted destructor. Third Error about G’s explicitly declared destructor inheriting F’s now implicitly deleted destructor. C++98: error: field of type 'NoDestroy' has private destructor note: implicitly declared private here note: implicit destructor for 'E' first required here C++11: error: deleted function '~E' cannot override a non-deleted function note: overridden virtual function is here error: non-deleted function '~F' cannot override a deleted function note: overridden virtual function is her
r294065 - Driver: Do not warn about unused -pthread when linking on darwin
Author: matze Date: Fri Feb 3 17:09:31 2017 New Revision: 294065 URL: http://llvm.org/viewvc/llvm-project?rev=294065&view=rev Log: Driver: Do not warn about unused -pthread when linking on darwin While there is nothing to do at link time to get pthreads support on darwin, specifying the argument is fine and we should not warn about unused arguments. Added: cfe/trunk/test/Driver/darwin-ld-pthread.c Modified: cfe/trunk/lib/Driver/Tools.cpp Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=294065&r1=294064&r2=294065&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cfe/trunk/lib/Driver/Tools.cpp Fri Feb 3 17:09:31 2017 @@ -8696,6 +8696,10 @@ void darwin::Linker::ConstructJob(Compil // Let the tool chain choose which runtime library to link. getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs); + +// No need to do anything for pthreads. Claim argument to avoid warning. +Args.ClaimAllArgs(options::OPT_pthread); +Args.ClaimAllArgs(options::OPT_pthreads); } if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) { Added: cfe/trunk/test/Driver/darwin-ld-pthread.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld-pthread.c?rev=294065&view=auto == --- cfe/trunk/test/Driver/darwin-ld-pthread.c (added) +++ cfe/trunk/test/Driver/darwin-ld-pthread.c Fri Feb 3 17:09:31 2017 @@ -0,0 +1,4 @@ +// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o /dev/null 2>&1 | FileCheck %s + +// There is nothing to do at link time to get pthread support. But do not warn. +// CHECK-NOT: argument unused during compilation: '-pthread' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28346: [AVR] Allow specifying the CPU on the command line
dylanmckay added a comment. Ping https://reviews.llvm.org/D28346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28344: [AVR] Add support for the full set of inline asm constraints
dylanmckay marked an inline comment as done. dylanmckay added a comment. Ping https://reviews.llvm.org/D28344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28344: [AVR] Add support for the full set of inline asm constraints
dylanmckay marked an inline comment as done. dylanmckay added a comment. Ping https://reviews.llvm.org/D28344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28451: [AVR] Add support for the 'interrupt' and 'naked' attributes
dylanmckay updated this revision to Diff 87054. dylanmckay marked 5 inline comments as done. dylanmckay added a comment. Code review comments - Add 'Subjects = [ObjCMethod]' to attributes - Use 'auto' keyword in one place - Move complex attr parsing logic into static function https://reviews.llvm.org/D28451 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td lib/CodeGen/TargetInfo.cpp lib/Sema/SemaDeclAttr.cpp test/CodeGen/avr/attributes/interrupt.c test/CodeGen/avr/attributes/signal.c test/Sema/avr-interrupt-attr.c test/Sema/avr-signal-attr.c Index: test/Sema/avr-signal-attr.c === --- /dev/null +++ test/Sema/avr-signal-attr.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 %s -triple avr-unknown-unknown -verify -fsyntax-only +struct a { int b; }; + +struct a test __attribute__((signal)); // expected-warning {{'signal' attribute only applies to functions and methods}} + +__attribute__((signal)) void food() {} Index: test/Sema/avr-interrupt-attr.c === --- /dev/null +++ test/Sema/avr-interrupt-attr.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 %s -triple avr-unknown-unknown -verify -fsyntax-only +struct a { int b; }; + +struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions and methods}} + +__attribute__((interrupt)) void food() {} Index: test/CodeGen/avr/attributes/signal.c === --- /dev/null +++ test/CodeGen/avr/attributes/signal.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm %s -o - | FileCheck %s + +// CHECK: define void @foo() #0 +__attribute__((signal)) void foo(void) { } + +// CHECK: attributes #0 = {{{.*signal.*}}} Index: test/CodeGen/avr/attributes/interrupt.c === --- /dev/null +++ test/CodeGen/avr/attributes/interrupt.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm %s -o - | FileCheck %s + +// CHECK: define void @foo() #0 +__attribute__((interrupt)) void foo(void) { } + +// CHECK: attributes #0 = {{{.*interrupt.*}}} Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -5126,6 +5126,26 @@ D->addAttr(UsedAttr::CreateImplicit(S.Context)); } +static void handleAVRInterruptAttr(Sema &S, Decl *D, const AttributeList &Attr) { + if (!isFunctionOrMethod(D)) { +S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) +<< "'interrupt'" << ExpectedFunctionOrMethod; +return; + } + + handleSimpleAttribute(S, D, Attr); +} + +static void handleAVRSignalAttr(Sema &S, Decl *D, const AttributeList &Attr) { + if (!isFunctionOrMethod(D)) { +S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) +<< "'signal'" << ExpectedFunctionOrMethod; +return; + } + + handleSimpleAttribute(S, D, Attr); +} + static void handleInterruptAttr(Sema &S, Decl *D, const AttributeList &Attr) { // Dispatch the interrupt attribute based on the current target. switch (S.Context.getTargetInfo().getTriple().getArch()) { @@ -5140,6 +5160,9 @@ case llvm::Triple::x86_64: handleAnyX86InterruptAttr(S, D, Attr); break; + case llvm::Triple::avr: +handleAVRInterruptAttr(S, D, Attr); +break; default: handleARMInterruptAttr(S, D, Attr); break; @@ -5700,6 +5723,9 @@ case AttributeList::AT_AMDGPUNumVGPR: handleAMDGPUNumVGPRAttr(S, D, Attr); break; + case AttributeList::AT_AVRSignal: +handleAVRSignalAttr(S, D, Attr); +break; case AttributeList::AT_IBAction: handleSimpleAttribute(S, D, Attr); break; Index: lib/CodeGen/TargetInfo.cpp === --- lib/CodeGen/TargetInfo.cpp +++ lib/CodeGen/TargetInfo.cpp @@ -6900,6 +6900,31 @@ } //===--===// +// AVR ABI Implementation. +//===--===// + +namespace { +class AVRTargetCodeGenInfo : public TargetCodeGenInfo { +public: + AVRTargetCodeGenInfo(CodeGenTypes &CGT) +: TargetCodeGenInfo(new DefaultABIInfo(CGT)) { } + + void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV, + CodeGen::CodeGenModule &CGM) const override { +const auto *FD = dyn_cast_or_null(D); +if (!FD) return; +auto *Fn = cast(GV); + +if (FD->getAttr()) + Fn->addFnAttr("interrupt"); + +if (FD->getAttr()) + Fn->addFnAttr("signal"); + } +}; +} + +//===--===// // TCE ABI Implementation (see http://tce.cs.tut.fi). Uses mostly the defaults. // Currently subclassed only to implement custom OpenCL C function at
r294078 - PR31846: Don't replace 'auto' type with a template parameter type in a generic lambda
Author: rsmith Date: Fri Feb 3 19:28:01 2017 New Revision: 294078 URL: http://llvm.org/viewvc/llvm-project?rev=294078&view=rev Log: PR31846: Don't replace 'auto' type with a template parameter type in a generic lambda until after we've checked whether 'auto' is valid in the current language mode. Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/test/SemaCXX/auto-cxx0x.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=294078&r1=294077&r2=294078&view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb 3 19:28:01 2017 @@ -6727,6 +6727,9 @@ public: /// \brief Substitute Replacement for auto in TypeWithAuto TypeSourceInfo* SubstAutoTypeSourceInfo(TypeSourceInfo *TypeWithAuto, QualType Replacement); + /// \brief Completely replace the \c auto in \p TypeWithAuto by + /// \p Replacement. This does not retain any \c auto type sugar. + QualType ReplaceAutoType(QualType TypeWithAuto, QualType Replacement); /// \brief Result type of DeduceAutoType. enum DeduceAutoResult { Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=294078&r1=294077&r2=294078&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Fri Feb 3 19:28:01 2017 @@ -4250,6 +4250,13 @@ TypeSourceInfo* Sema::SubstAutoTypeSourc .TransformType(TypeWithAuto); } +QualType Sema::ReplaceAutoType(QualType TypeWithAuto, + QualType TypeToReplaceAuto) { + return SubstituteAutoTransform(*this, TypeToReplaceAuto, + /*UseAutoSugar*/ false) + .TransformType(TypeWithAuto); +} + void Sema::DiagnoseAutoDeductionFailure(VarDecl *VDecl, Expr *Init) { if (isa(Init)) Diag(VDecl->getLocation(), Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=294078&r1=294077&r2=294078&view=diff == --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Fri Feb 3 19:28:01 2017 @@ -1502,40 +1502,7 @@ static QualType ConvertDeclSpecToType(Ty break; case DeclSpec::TST_auto: -// TypeQuals handled by caller. -// If auto is mentioned in a lambda parameter context, convert it to a -// template parameter type immediately, with the appropriate depth and -// index, and update sema's state (LambdaScopeInfo) for the current lambda -// being analyzed (which tracks the invented type template parameter). -if (declarator.getContext() == Declarator::LambdaExprParameterContext) { - sema::LambdaScopeInfo *LSI = S.getCurLambda(); - assert(LSI && "No LambdaScopeInfo on the stack!"); - const unsigned TemplateParameterDepth = LSI->AutoTemplateParameterDepth; - const unsigned AutoParameterPosition = LSI->AutoTemplateParams.size(); - const bool IsParameterPack = declarator.hasEllipsis(); - - // Turns out we must create the TemplateTypeParmDecl here to - // retrieve the corresponding template parameter type. - TemplateTypeParmDecl *CorrespondingTemplateParam = -TemplateTypeParmDecl::Create(Context, -// Temporarily add to the TranslationUnit DeclContext. When the -// associated TemplateParameterList is attached to a template -// declaration (such as FunctionTemplateDecl), the DeclContext -// for each template parameter gets updated appropriately via -// a call to AdoptTemplateParameterList. -Context.getTranslationUnitDecl(), -/*KeyLoc*/ SourceLocation(), -/*NameLoc*/ declarator.getLocStart(), -TemplateParameterDepth, -AutoParameterPosition, // our template param index -/* Identifier*/ nullptr, false, IsParameterPack); - LSI->AutoTemplateParams.push_back(CorrespondingTemplateParam); - // Replace the 'auto' in the function parameter with this invented - // template type parameter. - Result = QualType(CorrespondingTemplateParam->getTypeForDecl(), 0); -} else { - Result = Context.getAutoType(QualType(), AutoTypeKeyword::Auto, false); -} +Result = Context.getAutoType(QualType(), AutoTypeKeyword::Auto, false); break; case DeclSpec::TST_auto_type: @@ -2802,6 +2769,32 @@ static QualType GetDeclSpecTypeForDeclar if (!SemaRef.getLangOpts().CPlusPlus14 || !Auto || Auto->getKeyword() != AutoTypeKeyword::Auto) E
LLVM buildmaster will be updated and restarted tonight
Hello everyone, LLVM buildmaster will be updated and restarted after 7 PM Pacific time today. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29530: [ubsan] Reduce null checking of C++ object pointers (PR27581)
vsk created this revision. Given a load of a member variable from an instance method ('this->x'), ubsan inserts a null check for 'this', and another null check for '&this->x', before allowing the load to occur. Both of these checks are redundant, because 'this' must have been null-checked before the method is called. Similarly, given a call to a method from another method bound to the same instance ('this->foo()'), ubsan inserts a redundant null check for 'this'. There is also a redundant null check in the case where the object pointer is a reference ('Ref.foo()'). This patch teaches ubsan to remove the redundant null checks identified above. I'm not sure I've gone about this in the way suggested in PR27581, and would appreciate any advice/corrections. Testing: check-clang and check-ubsan. I also compiled X86FastISel.cpp with -fsanitize=null using patched/unpatched clangs based on r293572. Here are the number of null checks emitted in various setups: | Setup | # of null checks | | unpatched, -O0 | 21767| | unpatched, -O3 | 11862| | patched, -O0 | 13178| | patched, -O3 | 5547 | https://reviews.llvm.org/D29530 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-suppress-null-checks.cpp Index: test/CodeGenCXX/ubsan-suppress-null-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-suppress-null-checks.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=null | FileCheck %s + +struct A { + int foo; + + // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func1Ev + int func1() { +// CHECK-NOT: ubsan_handler_type_mismatch +return foo; + } + + // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func2Ev + int func2() { +// CHECK-NOT: ubsan_handler_type_mismatch +return func1(); + } + + // CHECK-LABEL: define linkonce_odr void @_ZN1A5func3Ev + void func3() { +// CHECK-NOT: ubsan_handler_type_mismatch +foo = 0; + } + + // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func4ERS_ + static int func4(A &a) { +// CHECK-NOT: ubsan_handler_type_mismatch +return a.func1(); + } +}; + +// Force clang to emit IR for A's methods. +void bar() { + A *a; + a->func1(); + a->func2(); + a->func3(); + A::func4(*a); +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -290,10 +290,16 @@ if (CE) CallLoc = CE->getExprLoc(); - EmitTypeCheck(isa(CalleeDecl) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + bool SkipNullCheck = false; + if (const auto *CMCE = dyn_cast(CE)) { +Expr *IOA = CMCE->getImplicitObjectArgument(); +SkipNullCheck = isa(IOA) || isa(IOA); + } + EmitTypeCheck( + isa(CalleeDecl) ? CodeGenFunction::TCK_ConstructorCall + : CodeGenFunction::TCK_MemberCall, + CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()), + /*Alignment=*/CharUnits::Zero(), SkipNullCheck); // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -953,9 +953,13 @@ LV = EmitArraySubscriptExpr(cast(E), /*Accessed*/true); else LV = EmitLValue(E); - if (!isa(E) && !LV.isBitField() && LV.isSimple()) + if (!isa(E) && !LV.isBitField() && LV.isSimple()) { +bool SkipNullCheck = false; +if (const auto *ME = dyn_cast(E)) + SkipNullCheck = isa(ME->getBase()); EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(), - E->getType(), LV.getAlignment()); + E->getType(), LV.getAlignment(), SkipNullCheck); + } return LV; } @@ -3335,7 +3339,9 @@ AlignmentSource AlignSource; Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource); QualType PtrTy = BaseExpr->getType()->getPointeeType(); -EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy); +bool SkipNullCheck = isa(BaseExpr); +EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy, + /*Alignment=*/CharUnits::Zero(), SkipNullCheck); BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource); } else BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess); Index: test/CodeGenCXX/ubsan-suppress-null-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-suppress-null-checks.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitiz