[PATCH] D33440: clang-format: better handle statement macros
Typz added inline comments. Comment at: lib/Format/FormatTokenLexer.cpp:642 tok::pp_define) && -std::find(ForEachMacros.begin(), ForEachMacros.end(), - FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) { - FormatTok->Type = TT_ForEachMacro; +(it = std::find(Macros.begin(), Macros.end(), +FormatTok->Tok.getIdentifierInfo())) != Macros.end()) { djasper wrote: > This does a binary search. Why aren't you implementing it with a hashtable? It was already done this way, so I did not change it to avoid any impact on performance. But I can change it if you prefer. https://reviews.llvm.org/D33440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33440: clang-format: better handle statement macros
Typz updated this revision to Diff 115801. Typz marked 5 inline comments as done. Typz added a comment. Add tests. Replace sorted list with hashtable. https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/FormatTokenLexer.h lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2385,6 +2385,42 @@ getLLVMStyleWithColumns(40))); verifyFormat("MACRO(>)"); + + // Some macros contain an implicit semicolon + FormatStyle Style = getLLVMStyle(); + Style.StatementMacros.push_back("FOO"); + verifyFormat("FOO(a) int b = 0;"); + verifyFormat("FOO(a)\n" + "int b = 0;", + Style); + verifyFormat("FOO(a);\n" + "int b = 0;", + Style); + verifyFormat("FOO(argc, argv, \"4.0.2\")\n" + "int b = 0;", + Style); + verifyFormat("FOO()\n" + "int b = 0;", + Style); + verifyFormat("FOO\n" + "int b = 0;", + Style); + verifyFormat("void f() {\n" + " FOO(a)\n" + " return a;\n" + "}", + Style); + verifyFormat("FOO(a)\n" + "FOO(b)", + Style); + verifyFormat("int a = 0;\n" + "FOO(b)\n" + "int c = 0;", + Style); + verifyFormat("int a = 0;\n" + "int x = FOO(a)\n" + "int b = 0;", + Style); } TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) { @@ -10185,6 +10221,12 @@ CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros, BoostAndQForeach); + Style.StatementMacros.clear(); + CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros, + std::vector{"QUNUSED"}); + CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros, + std::vector({"QUNUSED", "QT_REQUIRE_VERSION"})); + Style.IncludeCategories.clear(); std::vector ExpectedCategories = {{"abc/.*", 2}, {".*", 1}}; Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -1089,6 +1089,15 @@ return; } } +if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) { + nextToken(); + if (FormatTok->is(tok::l_paren)) +parseParens(); + if (FormatTok->is(tok::semi)) +nextToken(); + addUnwrappedLine(); + return; +} // In all other cases, parse the declaration. break; default: @@ -1238,6 +1247,16 @@ return; } + if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) { +nextToken(); +if (FormatTok->is(tok::l_paren)) + parseParens(); +if (FormatTok->is(tok::semi)) + nextToken(); +addUnwrappedLine(); +return; + } + // See if the following token should start a new unwrapped line. StringRef Text = FormatTok->TokenText; nextToken(); Index: lib/Format/FormatTokenLexer.h === --- lib/Format/FormatTokenLexer.h +++ lib/Format/FormatTokenLexer.h @@ -22,6 +22,7 @@ #include "clang/Basic/SourceManager.h" #include "clang/Format/Format.h" #include "llvm/Support/Regex.h" +#include #include @@ -97,7 +98,8 @@ // Index (in 'Tokens') of the last token that starts a new line. unsigned FirstInLineIndex; SmallVector Tokens; - SmallVector ForEachMacros; + + llvm::SmallMapVector Macros; bool FormattingDisabled; Index: lib/Format/FormatTokenLexer.cpp === --- lib/Format/FormatTokenLexer.cpp +++ lib/Format/FormatTokenLexer.cpp @@ -37,8 +37,9 @@ Lex->SetKeepWhitespaceMode(true); for (const std::string &ForEachMacro : Style.ForEachMacros) -ForEachMacros.push_back(&IdentTable.get(ForEachMacro)); - std::sort(ForEachMacros.begin(), ForEachMacros.end()); +Macros.insert({&IdentTable.get(ForEachMacro), TT_ForEachMacro}); + for (const std::string &StatementMacro : Style.StatementMacros) +Macros.insert({&IdentTable.get(StatementMacro), TT_StatementMacro}); } ArrayRef FormatTokenLexer::lex() { @@ -633,12 +634,13 @@ } if (Style.isCpp()) { +decltype(Macros)::iterator it; if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() && Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() == tok::pp_define) && -std::find(ForEachMacros.begin(), ForEachMacros.end(), -
[PATCH] D37813: clang-format: better handle namespace macros
Typz updated this revision to Diff 115802. Typz added a comment. rebase https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/TokenAnnotator.cpp lib/Format/UnwrappedLineFormatter.cpp lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp unittests/Format/NamespaceEndCommentsFixerTest.cpp Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp === --- unittests/Format/NamespaceEndCommentsFixerTest.cpp +++ unittests/Format/NamespaceEndCommentsFixerTest.cpp @@ -53,6 +53,7 @@ " int i;\n" " int j;\n" "}")); + EXPECT_EQ("namespace {\n" " int i;\n" " int j;\n" @@ -249,6 +250,85 @@ "// unrelated")); } +TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) { + FormatStyle Style = getLLVMStyle(); + Style.NamespaceMacros.push_back("TESTSUITE"); + + EXPECT_EQ("TESTSUITE() {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE()", +fixNamespaceEndComments("TESTSUITE() {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + + EXPECT_EQ("TESTSUITE(A) {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(A)", +fixNamespaceEndComments("TESTSUITE(A) {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + EXPECT_EQ("inline TESTSUITE(A) {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(A)", +fixNamespaceEndComments("inline TESTSUITE(A) {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + EXPECT_EQ("TESTSUITE(::A) {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(::A)", +fixNamespaceEndComments("TESTSUITE(::A) {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + EXPECT_EQ("TESTSUITE(::A::B) {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(::A::B)", +fixNamespaceEndComments("TESTSUITE(::A::B) {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(::A::B)", +fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + EXPECT_EQ("TESTSUITE(A, B) {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(A)", +fixNamespaceEndComments("TESTSUITE(A, B) {\n" +" int i;\n" +" int j;\n" +"}", +Style)); + EXPECT_EQ("TESTSUITE(\"Test1\") {\n" +" int i;\n" +" int j;\n" +"}// TESTSUITE(\"Test1\")", +fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n" +" int i;\n" +" int j;\n" +"}", +Style)); +} + TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) { EXPECT_EQ("namespace A {\n" " int i;\n" @@ -381,6 +461,54 @@ "}; /* unnamed namespace */")); } +TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) { + FormatStyle Style = getLLVMStyle(); + Style.NamespaceMacros.push_back("TESTSUITE"); + + EXPECT_EQ("TESTSUITE() {\n" +" int i;\n" +"} // end anonymous TESTSUITE()", +fixNamespaceEndComments("TESTSUITE() {\n" +" int i;\n" +"} // end anonymous TESTSUITE()", +Style)); + EXPECT_EQ("TESTSUITE(A) {\n" +" int i;\n" +"} /* end of TESTSUITE(A) */", +
Re: r310983 - PR19668, PR23034: Fix handling of move constructors and deleted copy
On 16 August 2017 at 02:49, Richard Smith via cfe-commits wrote: > + /// \brief \c true if a defaulted destructor for this class would be > deleted. > + bool defaultedDestructorIsDeleted() const { > +return !data().DefaultedDestructorIsDeleted; > + } Is the ! intentional? -- Malcolm Parsons ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings
klimek added a comment. So we actually didn't need to change the UnwrappedLineParser? I assume we still incorrectly assume the structured bindings are labmdas then? https://reviews.llvm.org/D35743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings
curdeius added a comment. There is one big missing thing here: `PointerAlignment`. Actually only `PAS_Left` is taken into account. There are 3 possible options: Left: `auto& [a, b] = f();` Middle: `auto & [a, b] = f();` Right: `auto &[a, b] = f();` https://reviews.llvm.org/D35743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.
curdeius updated this revision to Diff 115814. curdeius added a comment. Minor: use `FormatToken::isNot` instead of `!FormatToken::is`. https://reviews.llvm.org/D37980 Files: lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -1413,7 +1413,7 @@ verifyFormat("typedef enum {} EmptyEnum;"); verifyFormat("typedef enum { A, B, C } ShortEnum;"); verifyFormat("typedef enum {\n" - " ZERO = 0,\n" + " ZERO = 0,\n" " ONE = 1,\n" " TWO = 2,\n" " THREE = 3\n" @@ -1425,7 +1425,7 @@ verifyFormat("typedef enum { A, B, C } ShortEnum;"); verifyFormat("typedef enum\n" "{\n" - " ZERO = 0,\n" + " ZERO = 0,\n" " ONE = 1,\n" " TWO = 2,\n" " THREE = 3\n" @@ -1574,8 +1574,8 @@ Style.CompactNamespaces = true; verifyFormat("namespace A { namespace B {\n" - "}} // namespace A::B", - Style); + "}} // namespace A::B", + Style); EXPECT_EQ("namespace out { namespace in {\n" "}} // namespace out::in", @@ -3428,22 +3428,22 @@ verifyFormat( "SomeClass::Constructor() :\n" "a(aa), aaa() {}", - Style); + Style); verifyFormat( "SomeClass::Constructor() :\n" "a(aa), a(aa),\n" "a(aa) {}", - Style); + Style); verifyFormat( "SomeClass::Constructor() :\n" "aa(aa),\n" "aaa() {}", - Style); + Style); verifyFormat("Constructor(aa ,\n" "aa ) :\n" "aa(aa) {}", - Style); + Style); verifyFormat("Constructor() :\n" "(aaa),\n" @@ -3450,17 +3450,17 @@ "(aaa,\n" " aaa),\n" "aaa() {}", - Style); + Style); verifyFormat("Constructor() :\n" "(\n" "a) {}", - Style); + Style); verifyFormat("Constructor(int Parameter = 0) :\n" "aa(a),\n" "(a) {}", - Style); + Style); verifyFormat("Constructor() :\n" "aa(a), (b) {\n" "}", @@ -3468,7 +3468,7 @@ verifyFormat("Constructor() :\n" "a(\n" "a(, )) {}", - Style); + Style); // Here a line could be saved by splitting the second initializer onto two // lines, but that is not desirable. @@ -3476,7 +3476,7 @@ "(),\n" "aaa(aaa),\n" "at() {}", - Style); + Style); FormatStyle OnePerLine = Style; OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true; @@ -3526,7 +3526,7 @@ format("Constructor() :\n" "// Comment forcing unwanted break.\n" "() {}", - Style)); + Style)); Style.ColumnLimit = 0; verifyFormat("SomeClass::Constructor() :\n" @@ -3536,7 +3536,7 @@ "a(a) {}", Style); verifyFormat("SomeClass::Constructor() :\n" - "a(a), b(b), c(c) {}", + "a(a), b(b), c(c) {}", Style); verifyFormat("SomeClass::Constructor() :\n" "a(a) {\n" @@ -3547,12 +3547,12 @@ Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None; verifyFormat("SomeClass::Constructor() :\n" - "a(a), b(b), c(c) {\n" - "}", + "a(a), b(b), c(c) {\n" + "}", Style); verifyFormat("SomeClass::Constructor() :\n" "a(a) {\n" - "}", + "}", Style); Style.ColumnLimit = 80; @@ -6519,7 +6519,7 @@
r313622 - Fix formatting of lambda introducers with initializers.
Author: klimek Date: Tue Sep 19 02:59:30 2017 New Revision: 313622 URL: http://llvm.org/viewvc/llvm-project?rev=313622&view=rev Log: Fix formatting of lambda introducers with initializers. Most of the work was already done when we introduced a look-behind based lambda introducer detection. This patch finishes the transition by completely relying on the simple lambda introducer detection and simply recursing into normal brace-parsing code to parse until the end of the introducer. This fixes initializers in lambdas, including nested lambdas. Before: auto a = [b = [c = 42]{}]{}; auto b = [c = &i + 23]{}; After: auto a = [b = [c = 42] {}] {}; auto b = [c = &i + 23] {}; Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/lib/Format/UnwrappedLineParser.h cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=313622&r1=313621&r2=313622&view=diff == --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original) +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Sep 19 02:59:30 2017 @@ -1314,14 +1314,6 @@ bool UnwrappedLineParser::tryToParseLamb nextToken(); return false; } - const FormatToken* Previous = getPreviousToken(); - if (Previous && - (Previous->isOneOf(tok::identifier, tok::kw_operator, tok::kw_new, - tok::kw_delete) || - Previous->closesScope() || Previous->isSimpleTypeSpecifier())) { -nextToken(); -return false; - } assert(FormatTok->is(tok::l_square)); FormatToken &LSquare = *FormatTok; if (!tryToParseLambdaIntroducer()) @@ -1364,49 +1356,17 @@ bool UnwrappedLineParser::tryToParseLamb } bool UnwrappedLineParser::tryToParseLambdaIntroducer() { - nextToken(); - if (FormatTok->is(tok::equal)) { -nextToken(); -if (FormatTok->is(tok::r_square)) { - nextToken(); - return true; -} -if (FormatTok->isNot(tok::comma)) - return false; -nextToken(); - } else if (FormatTok->is(tok::amp)) { -nextToken(); -if (FormatTok->is(tok::r_square)) { - nextToken(); - return true; -} -if (!FormatTok->isOneOf(tok::comma, tok::identifier)) { - return false; -} -if (FormatTok->is(tok::comma)) - nextToken(); - } else if (FormatTok->is(tok::r_square)) { + const FormatToken* Previous = getPreviousToken(); + if (Previous && + (Previous->isOneOf(tok::identifier, tok::kw_operator, tok::kw_new, + tok::kw_delete) || + Previous->closesScope() || Previous->isSimpleTypeSpecifier())) { nextToken(); -return true; +return false; } - do { -if (FormatTok->is(tok::amp)) - nextToken(); -if (!FormatTok->isOneOf(tok::identifier, tok::kw_this)) - return false; -nextToken(); -if (FormatTok->is(tok::ellipsis)) - nextToken(); -if (FormatTok->is(tok::comma)) { - nextToken(); -} else if (FormatTok->is(tok::r_square)) { - nextToken(); - return true; -} else { - return false; -} - } while (!eof()); - return false; + nextToken(); + parseSquare(/*LambdaIntroducer=*/true); + return true; } void UnwrappedLineParser::tryToParseJSFunction() { @@ -1608,10 +1568,12 @@ void UnwrappedLineParser::parseParens() } while (!eof()); } -void UnwrappedLineParser::parseSquare() { - assert(FormatTok->Tok.is(tok::l_square) && "'[' expected."); - if (tryToParseLambda()) -return; +void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) { + if (!LambdaIntroducer) { +assert(FormatTok->Tok.is(tok::l_square) && "'[' expected."); +if (tryToParseLambda()) + return; + } do { switch (FormatTok->Tok.getKind()) { case tok::l_paren: Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=313622&r1=313621&r2=313622&view=diff == --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original) +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Tue Sep 19 02:59:30 2017 @@ -96,7 +96,7 @@ private: bool parseBracedList(bool ContinueOnSemicolons = false, tok::TokenKind ClosingBraceKind = tok::r_brace); void parseParens(); - void parseSquare(); + void parseSquare(bool LambdaIntroducer = false); void parseIfThenElse(); void parseTryCatch(); void parseForOrWhileLoop(); Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=313622&r1=313621&r2=313622&view=diff == --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Sep 19 02:59:30 2
[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.
klimek added a comment. Thanks for the patch and the discussion around this. I fixed this in r313622 in what I think is a more principled approach that also works for nested lambdas (and gets rid of a lot of now-obsolete code). The big problem with this code was that it evolved a bit to the point where we were able to generalize it, but we never did. Let me know if you find any problems with the aforementioned change. https://reviews.llvm.org/D37980 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38032: [clangd] Serialize onDiagnosticsReady callbacks for the same file.
ilya-biryukov created this revision. Calls to onDiagnosticsReady were done concurrently before. This sometimes led to older versions of diagnostics being reported to the user after the newer versions. https://reviews.llvm.org/D38032 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/DraftStore.h unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace clang { @@ -892,5 +893,67 @@ } } +TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { + class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer { + public: +NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) +: StartSecondReparse(std::move(StartSecondReparse)) {} + +void onDiagnosticsReady( +PathRef File, +Tagged> Diagnostics) override { + + std::unique_lock Lock(Mutex, std::try_to_lock_t()); + ASSERT_TRUE(Lock.owns_lock()) + << "Detected concurrent onDiagnosticsReady calls for the same file."; + if (FirstRequest) { +FirstRequest = false; +StartSecondReparse.set_value(); +// Sleep long enough for the second request to be processed. +std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + Mutex.unlock(); +} + + private: +std::mutex Mutex; +bool FirstRequest = true; +std::promise StartSecondReparse; + }; + + const auto SourceContentsWithoutErrors = R"cpp( +int a; +int b; +int c; +int d; +)cpp"; + + const auto SourceContentsWithErrors = R"cpp( +int a = x; +int b; +int c; +int d; +)cpp"; + + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + llvm::StringMap FileContents; + FileContents[FooCpp] = ""; + ConstantFSProvider FS(buildTestFS(FileContents)); + + std::promise StartSecondReparsePromise; + std::future StartSecondReparse = StartSecondReparsePromise.get_future(); + + NoConcurrentAccessDiagConsumer DiagConsumer( + std::move(StartSecondReparsePromise)); + + MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); + ClangdServer Server(CDB, DiagConsumer, FS, 4, /*SnippetCompletions=*/false); + Server.addDocument(FooCpp, SourceContentsWithErrors); + StartSecondReparse.wait(); + + auto Future = Server.addDocument(FooCpp, SourceContentsWithoutErrors); + Future.wait(); +} + } // namespace clangd } // namespace clang Index: clangd/DraftStore.h === --- clangd/DraftStore.h +++ clangd/DraftStore.h @@ -13,15 +13,16 @@ #include "Path.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/StringMap.h" +#include #include #include #include namespace clang { namespace clangd { -/// Using 'unsigned' here to avoid undefined behaviour on overflow. -typedef unsigned DocVersion; +/// Using unsigned int type here to avoid undefined behaviour on overflow. +typedef uint64_t DocVersion; /// Document draft with a version of this draft. struct VersionedDraft { Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -279,6 +279,12 @@ // ClangdServer ClangdScheduler WorkScheduler; bool SnippetCompletions; + + /// Used to serialize diagnostic callbacks. + /// FIXME(ibiryukov): get rid of an extra map and put all version counters + /// into CppFile. + std::mutex DiagnosticsMutex; + llvm::StringMap ReportedDiagnosticVersions; }; } // namespace clangd Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -313,6 +313,19 @@ auto Diags = DeferredRebuild.get(); if (!Diags) return; // A new reparse was requested before this one completed. + +// We need to serialize access to resulting diagnostics to avoid calling +// `onDiagnosticsReady` in the wrong order. +std::lock_guard DiagsLock(DiagnosticsMutex); +DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr]; +// FIXME(ibiryukov): get rid of '<' comparison here. In the current +// implementation diagnostics will not be reported after version counters' +// overflow. This should not happen in practice, since DocVersion is a +// 64-bit unsigned integer. +if (Version < LastReportedDiagsVersion) + return; +LastReportedDiagsVersion = Version; + DiagConsumer.onDiagnosticsReady(FileStr, make_tagged(std::move(*Diags), Tag)); }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37263: [clang-format] Ignore case when sorting using-declarations
alexfh added a comment. As discussed offline, the sorting should also be stable to avoid no-op replacements for identical using declarations and randomizing the order of using declarations differing only in case (not that I'd expect this to be a frequent thing, but it's better to handle it properly anyway). https://reviews.llvm.org/D37263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37972: [clangd] Introduced Logger interface.
ilya-biryukov updated this revision to Diff 115818. ilya-biryukov added a comment. - Replaced JSONOutput in Protocol.h/Protocol.cpp with Logger. https://reviews.llvm.org/D37972 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/Protocol.cpp clangd/Protocol.h unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" +#include "Logger.h" #include "clang/Basic/VirtualFileSystem.h" #include "clang/Config/config.h" #include "llvm/ADT/SmallVector.h" @@ -302,7 +303,8 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), -/*SnippetCompletions=*/false); +/*SnippetCompletions=*/false, +EmptyLogger::getInstance()); for (const auto &FileWithContents : ExtraFiles) FS.Files[getVirtualTestFilePath(FileWithContents.first)] = FileWithContents.second; @@ -365,7 +367,7 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false); + /*SnippetCompletions=*/false, EmptyLogger::getInstance()); const auto SourceContents = R"cpp( #include "foo.h" @@ -410,7 +412,7 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false); + /*SnippetCompletions=*/false, EmptyLogger::getInstance()); const auto SourceContents = R"cpp( #include "foo.h" @@ -457,7 +459,8 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); // Run ClangdServer synchronously. ClangdServer Server(CDB, DiagConsumer, FS, - /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false); + /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false, + EmptyLogger::getInstance()); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = "int a;"; @@ -490,7 +493,8 @@ "-stdlib=libstdc++"}); // Run ClangdServer synchronously. ClangdServer Server(CDB, DiagConsumer, FS, - /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false); + /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false, + EmptyLogger::getInstance()); // Just a random gcc version string SmallString<8> Version("4.9.3"); @@ -538,7 +542,8 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false); + /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false, + EmptyLogger::getInstance()); // No need to sync reparses, because reparses are performed on the calling // thread to true. @@ -597,7 +602,7 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false); + /*SnippetCompletions=*/false, EmptyLogger::getInstance()); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -745,7 +750,8 @@ { MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), -/*SnippetCompletions=*/false); +/*SnippetCompletions=*/false, +EmptyLogger::getInstance()); // Prepare some random distributions for the test. std::random_device RandGen; Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -29,7 +29,7 @@ namespace clang { namespace clangd { -class JSONOutput; +class Logger; struct URI { std::string uri; @@ -59,7 +59,7 @@ URI uri; static llvm::Optional - parse(llvm::yaml::MappingNode *Params, JSONOutput &Output); + parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
[PATCH] D38032: [clangd] Serialize onDiagnosticsReady callbacks for the same file.
ilya-biryukov updated this revision to Diff 115821. ilya-biryukov added a comment. - Removed an incidental `Mutex.unlock()`, a leftover from previous drafts. https://reviews.llvm.org/D38032 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/DraftStore.h unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace clang { @@ -892,5 +893,66 @@ } } +TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { + class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer { + public: +NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) +: StartSecondReparse(std::move(StartSecondReparse)) {} + +void onDiagnosticsReady( +PathRef File, +Tagged> Diagnostics) override { + + std::unique_lock Lock(Mutex, std::try_to_lock_t()); + ASSERT_TRUE(Lock.owns_lock()) + << "Detected concurrent onDiagnosticsReady calls for the same file."; + if (FirstRequest) { +FirstRequest = false; +StartSecondReparse.set_value(); +// Sleep long enough for the second request to be processed. +std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } +} + + private: +std::mutex Mutex; +bool FirstRequest = true; +std::promise StartSecondReparse; + }; + + const auto SourceContentsWithoutErrors = R"cpp( +int a; +int b; +int c; +int d; +)cpp"; + + const auto SourceContentsWithErrors = R"cpp( +int a = x; +int b; +int c; +int d; +)cpp"; + + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + llvm::StringMap FileContents; + FileContents[FooCpp] = ""; + ConstantFSProvider FS(buildTestFS(FileContents)); + + std::promise StartSecondReparsePromise; + std::future StartSecondReparse = StartSecondReparsePromise.get_future(); + + NoConcurrentAccessDiagConsumer DiagConsumer( + std::move(StartSecondReparsePromise)); + + MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); + ClangdServer Server(CDB, DiagConsumer, FS, 4, /*SnippetCompletions=*/false); + Server.addDocument(FooCpp, SourceContentsWithErrors); + StartSecondReparse.wait(); + + auto Future = Server.addDocument(FooCpp, SourceContentsWithoutErrors); + Future.wait(); +} + } // namespace clangd } // namespace clang Index: clangd/DraftStore.h === --- clangd/DraftStore.h +++ clangd/DraftStore.h @@ -13,15 +13,16 @@ #include "Path.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/StringMap.h" +#include #include #include #include namespace clang { namespace clangd { -/// Using 'unsigned' here to avoid undefined behaviour on overflow. -typedef unsigned DocVersion; +/// Using unsigned int type here to avoid undefined behaviour on overflow. +typedef uint64_t DocVersion; /// Document draft with a version of this draft. struct VersionedDraft { Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -279,6 +279,12 @@ // ClangdServer ClangdScheduler WorkScheduler; bool SnippetCompletions; + + /// Used to serialize diagnostic callbacks. + /// FIXME(ibiryukov): get rid of an extra map and put all version counters + /// into CppFile. + std::mutex DiagnosticsMutex; + llvm::StringMap ReportedDiagnosticVersions; }; } // namespace clangd Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -313,6 +313,19 @@ auto Diags = DeferredRebuild.get(); if (!Diags) return; // A new reparse was requested before this one completed. + +// We need to serialize access to resulting diagnostics to avoid calling +// `onDiagnosticsReady` in the wrong order. +std::lock_guard DiagsLock(DiagnosticsMutex); +DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr]; +// FIXME(ibiryukov): get rid of '<' comparison here. In the current +// implementation diagnostics will not be reported after version counters' +// overflow. This should not happen in practice, since DocVersion is a +// 64-bit unsigned integer. +if (Version < LastReportedDiagsVersion) + return; +LastReportedDiagsVersion = Version; + DiagConsumer.onDiagnosticsReady(FileStr, make_tagged(std::move(*Diags), Tag)); }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r313624 - Lowering Mask Set1 intrinsics to LLVM IR
Author: jina.nahias Date: Tue Sep 19 04:00:27 2017 New Revision: 313624 URL: http://llvm.org/viewvc/llvm-project?rev=313624&view=rev Log: Lowering Mask Set1 intrinsics to LLVM IR This patch, together with a matching llvm patch (https://reviews.llvm.org/D37669), implements the lowering of X86 mask set1 intrinsics to IR. Differential Revision: https://reviews.llvm.org/D37668 Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/include/clang/Basic/BuiltinsX86_64.def cfe/trunk/lib/Headers/avx512bwintrin.h cfe/trunk/lib/Headers/avx512fintrin.h cfe/trunk/lib/Headers/avx512vlbwintrin.h cfe/trunk/lib/Headers/avx512vlintrin.h cfe/trunk/test/CodeGen/avx512bw-builtins.c cfe/trunk/test/CodeGen/avx512f-builtins.c cfe/trunk/test/CodeGen/avx512vl-builtins.c cfe/trunk/test/CodeGen/avx512vlbw-builtins.c Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=313624&r1=313623&r2=313624&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Sep 19 04:00:27 2017 @@ -973,7 +973,6 @@ TARGET_BUILTIN(__builtin_ia32_pmuldq512, TARGET_BUILTIN(__builtin_ia32_pmuludq512, "V8LLiV16iV16i", "", "avx512f") TARGET_BUILTIN(__builtin_ia32_ptestmd512, "UsV16iV16iUs", "", "avx512f") TARGET_BUILTIN(__builtin_ia32_ptestmq512, "UcV8LLiV8LLiUc", "", "avx512f") -TARGET_BUILTIN(__builtin_ia32_pbroadcastd512_gpr_mask, "V16iiV16iUs", "", "avx512f") TARGET_BUILTIN(__builtin_ia32_loaddqusi512_mask, "V16iiC*V16iUs", "", "avx512f") TARGET_BUILTIN(__builtin_ia32_loaddqudi512_mask, "V8LLiLLiC*V8LLiUc", "", "avx512f") TARGET_BUILTIN(__builtin_ia32_loadups512_mask, "V16ffC*V16fUs", "", "avx512f") @@ -1374,11 +1373,6 @@ TARGET_BUILTIN(__builtin_ia32_movdqa64lo TARGET_BUILTIN(__builtin_ia32_movdqa64load256_mask, "V4LLiV4LLiC*V4LLiUc","","avx512vl") TARGET_BUILTIN(__builtin_ia32_movdqa64store128_mask, "vV2LLi*V2LLiUc","","avx512f") TARGET_BUILTIN(__builtin_ia32_movdqa64store256_mask, "vV4LLi*V4LLiUc","","avx512f") -TARGET_BUILTIN(__builtin_ia32_pbroadcastb512_gpr_mask, "V64ccV64cULLi","","avx512bw") -TARGET_BUILTIN(__builtin_ia32_pbroadcastb128_gpr_mask, "V16ccV16cUs","","avx512bw,avx512vl") -TARGET_BUILTIN(__builtin_ia32_pbroadcastb256_gpr_mask, "V32ccV32cUi","","avx512bw,avx512vl") -TARGET_BUILTIN(__builtin_ia32_pbroadcastd128_gpr_mask, "V4iiV4iUc","","avx512vl") -TARGET_BUILTIN(__builtin_ia32_pbroadcastd256_gpr_mask, "V8iiV8iUc","","avx512vl") TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512_mask, "V8LLiV8LLiV8LLiV8LLiUc","","avx512ifma") TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512_maskz, "V8LLiV8LLiV8LLiV8LLiUc","","avx512ifma") TARGET_BUILTIN(__builtin_ia32_vpmadd52luq512_mask, "V8LLiV8LLiV8LLiV8LLiUc","","avx512ifma") @@ -1589,9 +1583,6 @@ TARGET_BUILTIN(__builtin_ia32_broadcastm TARGET_BUILTIN(__builtin_ia32_broadcastmb256, "V4LLiUc","","avx512cd,avx512vl") TARGET_BUILTIN(__builtin_ia32_broadcastmw128, "V4iUs","","avx512cd,avx512vl") TARGET_BUILTIN(__builtin_ia32_broadcastmw256, "V8iUs","","avx512cd,avx512vl") -TARGET_BUILTIN(__builtin_ia32_pbroadcastw512_gpr_mask, "V32shV32sUi","","avx512bw") -TARGET_BUILTIN(__builtin_ia32_pbroadcastw256_gpr_mask, "V16shV16sUs","","avx512bw,avx512vl") -TARGET_BUILTIN(__builtin_ia32_pbroadcastw128_gpr_mask, "V8ssV8sUc","","avx512bw,avx512vl") TARGET_BUILTIN(__builtin_ia32_pmovsdb512_mask, "V16cV16iV16cUs","","avx512f") TARGET_BUILTIN(__builtin_ia32_pmovsdb512mem_mask, "vV16c*V16iUs","","avx512f") TARGET_BUILTIN(__builtin_ia32_pmovswb512mem_mask, "vV32c*V32sUi","","avx512bw") Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=313624&r1=313623&r2=313624&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Tue Sep 19 04:00:27 2017 @@ -71,9 +71,6 @@ TARGET_BUILTIN(__builtin_ia32_pext_di, " TARGET_BUILTIN(__builtin_ia32_bextri_u64, "ULLiULLiIULLi", "", "tbm") TARGET_BUILTIN(__builtin_ia32_lwpins64, "UcULLiUiUi", "", "lwp") TARGET_BUILTIN(__builtin_ia32_lwpval64, "vULLiUiUi", "", "lwp") -TARGET_BUILTIN(__builtin_ia32_pbroadcastq512_gpr_mask, "V8LLiLLiV8LLiUc", "", "avx512f") -TARGET_BUILTIN(__builtin_ia32_pbroadcastq128_gpr_mask, "V2LLiULLiV2LLiUc","","avx512vl") -TARGET_BUILTIN(__builtin_ia32_pbroadcastq256_gpr_mask, "V4LLiULLiV4LLiUc","","avx512vl") TARGET_BUILTIN(__builtin_ia32_vcvtsd2si64, "LLiV2dIi","","avx512f") TARGET_BUILTIN(__builtin_ia32_vcvtsd2usi64, "ULLiV2dIi","","avx512f") TARGET_BUILTIN(__builtin_ia32_vcvtss2si64, "LLiV4fIi","","avx512f") Modified: cfe/trunk/lib/Headers/avx512bwintrin.h URL: http://llvm.org/viewvc/llvm-pro
[PATCH] D37668: [X86][intrinsics] lower _mm[256|512]_mask[z]_set1_epi[8|16|32|64] intrinsic to IR
This revision was automatically updated to reflect the committed changes. Closed by commit rL313624: Lowering Mask Set1 intrinsics to LLVM IR (authored by jina.nahias). Changed prior to commit: https://reviews.llvm.org/D37668?vs=115622&id=115823#toc Repository: rL LLVM https://reviews.llvm.org/D37668 Files: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/include/clang/Basic/BuiltinsX86_64.def cfe/trunk/lib/Headers/avx512bwintrin.h cfe/trunk/lib/Headers/avx512fintrin.h cfe/trunk/lib/Headers/avx512vlbwintrin.h cfe/trunk/lib/Headers/avx512vlintrin.h cfe/trunk/test/CodeGen/avx512bw-builtins.c cfe/trunk/test/CodeGen/avx512f-builtins.c cfe/trunk/test/CodeGen/avx512vl-builtins.c cfe/trunk/test/CodeGen/avx512vlbw-builtins.c Index: cfe/trunk/lib/Headers/avx512vlintrin.h === --- cfe/trunk/lib/Headers/avx512vlintrin.h +++ cfe/trunk/lib/Headers/avx512vlintrin.h @@ -5723,59 +5723,72 @@ (__v4df)_mm256_setzero_pd()); } +static __inline__ __m128i __DEFAULT_FN_ATTRS +_mm_mask_set1_epi32(__m128i __O, __mmask8 __M, int __A) +{ + return (__m128i)__builtin_ia32_selectd_128(__M, + (__v4si) _mm_set1_epi32(__A), + (__v4si)__O); +} + +static __inline__ __m128i __DEFAULT_FN_ATTRS +_mm_maskz_set1_epi32( __mmask8 __M, int __A) +{ + return (__m128i)__builtin_ia32_selectd_128(__M, + (__v4si) _mm_set1_epi32(__A), + (__v4si)_mm_setzero_si128()); +} + +static __inline__ __m256i __DEFAULT_FN_ATTRS +_mm256_mask_set1_epi32(__m256i __O, __mmask8 __M, int __A) +{ + return (__m256i)__builtin_ia32_selectd_256(__M, + (__v8si) _mm256_set1_epi32(__A), + (__v8si)__O); +} + +static __inline__ __m256i __DEFAULT_FN_ATTRS +_mm256_maskz_set1_epi32( __mmask8 __M, int __A) +{ + return (__m256i)__builtin_ia32_selectd_256(__M, + (__v8si) _mm256_set1_epi32(__A), + (__v8si)_mm256_setzero_si256()); +} -#define _mm_mask_set1_epi32(O, M, A) __extension__ ({ \ - (__m128i)__builtin_ia32_pbroadcastd128_gpr_mask((int)(A), \ - (__v4si)(__m128i)(O), \ - (__mmask8)(M)); }) - -#define _mm_maskz_set1_epi32(M, A) __extension__ ({ \ - (__m128i)__builtin_ia32_pbroadcastd128_gpr_mask((int)(A), \ - (__v4si)_mm_setzero_si128(), \ - (__mmask8)(M)); }) - -#define _mm256_mask_set1_epi32(O, M, A) __extension__ ({ \ - (__m256i)__builtin_ia32_pbroadcastd256_gpr_mask((int)(A), \ - (__v8si)(__m256i)(O), \ - (__mmask8)(M)); }) - -#define _mm256_maskz_set1_epi32(M, A) __extension__ ({ \ - (__m256i)__builtin_ia32_pbroadcastd256_gpr_mask((int)(A), \ - (__v8si)_mm256_setzero_si256(), \ - (__mmask8)(M)); }) #ifdef __x86_64__ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_mask_set1_epi64 (__m128i __O, __mmask8 __M, long long __A) { - return (__m128i) __builtin_ia32_pbroadcastq128_gpr_mask (__A, (__v2di) __O, - __M); + return (__m128i) __builtin_ia32_selectq_128(__M, + (__v2di) _mm_set1_epi8(__A), + (__v2di) __O); } static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_maskz_set1_epi64 (__mmask8 __M, long long __A) { - return (__m128i) __builtin_ia32_pbroadcastq128_gpr_mask (__A, - (__v2di) - _mm_setzero_si128 (), - __M); + return (__m128i) __builtin_ia32_selectq_128(__M, + (__v2di) _mm_set1_epi8(__A), + (__v2di) _mm_setzero_si128()); } static __inline__ __m256i __DEFAULT_FN_ATTRS _mm256_mask_set1_epi64 (__m256i __O, __mmask8 __M, long long __A) { - return (__m256i) __builtin_ia32_pbroadcastq256_gpr_mask (__A, (__v4di) __O, - __M); + return (__m256i) __builtin_ia32_selectq_256(__M, + (__v4di) _mm256_set1_epi64x(__A), + (__v4di) __O) ; } static __inline__ __m256i __DEFAULT_FN_ATTRS _mm256_maskz_set1_epi64 (__mmask8 __M, long long __A) { - return (__m256i) __builtin_ia32_pbroadcastq256_gpr_mask (__A, - (__v4di) - _mm256_setzero_si256 (), - __M); + return (__m256i) __builtin_i
[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.
bjope accepted this revision. bjope added a comment. This revision is now accepted and ready to land. Nobody else seems to bother. At least no objections, so I think we can land this now. https://reviews.llvm.org/D37254 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
Typz added a comment. For one thing, we need to update the state to store the "decision" of the reflowing mode, which is performed only in DryRun=true mode, to avoid doing the computation multiple times. Apart from this, the decision is conceptually internal to breakProtrudingToken(). But the 'computation' of either mode is very similar, and updates the state variable anyway (State.Column, Stack.back().LastSpace, State.Stack[i].BreakBeforeParameter, Token->updateNextToken(State)) : so it is simpler (code-wise) to split the function and call it twice, then decide which 'State' variable to use. https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
djasper added a comment. I think doing the computation twice is fine. Or at least, I'd need a test case where it actually shows substantial overhead before doing what you are doing here. Understand that creating more States and making the State object itself larger also has cost and that cost occurs in the combinatorial exploration of the solution space. Doing an additional computation at the end should be comparatively cheap. Look at it this way: During the exploration of the solution space, we might enter breakProtrudingToken many times for the same comment. One more time during reconstruction of the solution is not that harmful. https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 115827. werbitt added a comment. Clean up call-process-region - Quote call-process-region with #', this will cause a compile time error if call-process-region is undefined - Pass positional arguments normally (exclude from the list) - Instead of using append use a backquoted list - Use ,@ to splice the assume file logic into the list - Use 'and' instead of 'if' https://reviews.llvm.org/D37903 Files: tools/clang-format/clang-format.el Index: tools/clang-format/clang-format.el === --- tools/clang-format/clang-format.el +++ tools/clang-format/clang-format.el @@ -147,16 +147,15 @@ ;; always use ‘utf-8-unix’ and ignore the buffer coding system. (default-process-coding-system '(utf-8-unix . utf-8-unix))) (unwind-protect -(let ((status (apply 'call-process-region - (append `(nil nil ,clang-format-executable - nil (,temp-buffer ,temp-file) nil) - '("-output-replacements-xml") - (if assume-file - `("-assume-filename" ,assume-file) nil) - `("-style" ,style - "-offset" ,(number-to-string file-start) - "-length" ,(number-to-string (- file-end file-start)) - "-cursor" ,(number-to-string cursor) +(let ((status (apply #'call-process-region + nil nil clang-format-executable + nil `(,temp-buffer ,temp-file) nil + `("-output-replacements-xml" + ,@(and assume-file (list "-assume-filename" assume-file)) + "-style" ,style + "-offset" ,(number-to-string file-start) + "-length" ,(number-to-string (- file-end file-start)) + "-cursor" ,(number-to-string cursor (stderr (with-temp-buffer (unless (zerop (cadr (insert-file-contents temp-file))) (insert ": ")) Index: tools/clang-format/clang-format.el === --- tools/clang-format/clang-format.el +++ tools/clang-format/clang-format.el @@ -147,16 +147,15 @@ ;; always use ‘utf-8-unix’ and ignore the buffer coding system. (default-process-coding-system '(utf-8-unix . utf-8-unix))) (unwind-protect -(let ((status (apply 'call-process-region - (append `(nil nil ,clang-format-executable - nil (,temp-buffer ,temp-file) nil) - '("-output-replacements-xml") - (if assume-file - `("-assume-filename" ,assume-file) nil) - `("-style" ,style - "-offset" ,(number-to-string file-start) - "-length" ,(number-to-string (- file-end file-start)) - "-cursor" ,(number-to-string cursor) +(let ((status (apply #'call-process-region + nil nil clang-format-executable + nil `(,temp-buffer ,temp-file) nil + `("-output-replacements-xml" + ,@(and assume-file (list "-assume-filename" assume-file)) + "-style" ,style + "-offset" ,(number-to-string file-start) + "-length" ,(number-to-string (- file-end file-start)) + "-cursor" ,(number-to-string cursor (stderr (with-temp-buffer (unless (zerop (cadr (insert-file-contents temp-file))) (insert ": ")) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt added a comment. Hi, Thank you very much for your feedback. I submitted a bug here: https://bugs.llvm.org/show_bug.cgi?id=34667 I made the changes you suggested, but I left the assume-filename optional argument for now. My current use-case is that when I'm editing a source block in an orgmode file, the code is in new buffer that doesn't have a buffer-file-name. If I'm able to pass in the base orgmode buffer-file-name, I can apply the .clang-format file from the correct directory. Without it, I'll need to locate the .clang-format file and use it to populate the style argument, which would be a lot of extra code on my end. If you still think it's not worth it to have the assume-file argument let me know and I'll remove it. Thanks, Micah https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
Typz added a comment. In https://reviews.llvm.org/D33589#875039, @djasper wrote: > I think doing the computation twice is fine. Or at least, I'd need a test > case where it actually shows substantial overhead before doing what you are > doing here. Understand that creating more States and making the State object > itself larger also has cost and that cost occurs in the combinatorial > exploration of the solution space. Doing an additional computation at the end > should be comparatively cheap. Look at it this way: During the exploration of > the solution space, we might enter breakProtrudingToken many times for the > same comment. One more time during reconstruction of the solution is not that > harmful. I 've just tried to implement this (e.g. make the 2 calls also in DryRun), and I am running into an assertion in WhitespaceManager : [ RUN ] FormatTest.ConfigurableUseOfTab FormatTests: /workspace/llvm/tools/clang/lib/Format/WhitespaceManager.cpp:112: void clang::format::WhitespaceManager::calculateLineBreakInformation(): Assertion `PreviousOriginalWhitespaceEndOffset <= OriginalWhitespaceStartOffset' failed. This remind there was another "reason" for limiting this to DryRun (not sure it is a good or bad reason): it happens only in the optimizer, all other cases where the indenter is used are not affected. I am still trying to get to the bottom of this assertion, any hint where to look for? https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
Typz added a comment. > I am still trying to get to the bottom of this assertion, any hint where to > look for? OK, got it. The issue is that we will actually need to run the wrapping 3 times when DryRun = false : call reflowProtrudingToken() twice with DryRun=true to find out the better option, then call it once more with DryRun=false. https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37972: [clangd] Introduced Logger interface.
malaperle accepted this revision. malaperle added a comment. This revision is now accepted and ready to land. Look good. Thank you! https://reviews.llvm.org/D37972 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
Typz updated this revision to Diff 115830. Typz added a comment. Remove `Reflow` from LineState, and perform the decision again during reconstruction phase. https://reviews.llvm.org/D33589 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -9809,6 +9809,60 @@ EXPECT_EQ("#pragma option -C -A", format("#pragmaoption -C -A")); } +TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) { + FormatStyle Style = getLLVMStyle(); + Style.ColumnLimit = 20; + + verifyFormat("int a; // the\n" + " // comment", Style); + EXPECT_EQ("int a; /* first line\n" +"* second\n" +"* line third\n" +"* line\n" +"*/", + format("int a; /* first line\n" + "* second\n" + "* line third\n" + "* line\n" + "*/", Style)); + EXPECT_EQ("int a; // first line\n" +" // second\n" +" // line third\n" +" // line", +format("int a; // first line\n" + " // second line\n" + " // third line", + Style)); + + Style.PenaltyExcessCharacter = 90; + verifyFormat("int a; // the comment", Style); + EXPECT_EQ("int a; // the\n" +" // comment aa", +format("int a; // the comment aa", Style)); + EXPECT_EQ("int a; /* first line\n" +"* second line\n" +"* third line\n" +"*/", + format("int a; /* first line\n" + "* second line\n" + "* third line\n" + "*/", Style)); + EXPECT_EQ("int a; // first line\n" +" // second line\n" +" // third line", +format("int a; // first line\n" + " // second line\n" + " // third line", + Style)); + EXPECT_EQ("int a; /* first line\n" +"* second\n" +"* line third\n" +"* line\n" +"*/", +format("int a; /* first line second line third line" + "\n*/", Style)); +} + #define EXPECT_ALL_STYLES_EQUAL(Styles)\ for (size_t i = 1; i < Styles.size(); ++i) \ EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \ Index: lib/Format/ContinuationIndenter.h === --- lib/Format/ContinuationIndenter.h +++ lib/Format/ContinuationIndenter.h @@ -27,6 +27,7 @@ namespace format { class AnnotatedLine; +class BreakableToken; struct FormatToken; struct LineState; struct ParenState; @@ -100,6 +101,16 @@ unsigned breakProtrudingToken(const FormatToken &Current, LineState &State, bool DryRun); + /// \brief Perform the reflowing of a BreakableToken. + /// Reflow=true, then the function will reflow the token as needed. Otherwise, it simply computes + /// the penalty caused by this tokens characters. + /// + /// \returns The penalty of reflowing the token if State.Reflow=true; otherwise + /// the penalty of characters going beyond the column limit. + unsigned reflowProtrudingToken(const FormatToken & Current, LineState & State, + std::unique_ptr & Token, + unsigned ColumnLimit, bool DryRun, bool Reflow); + /// \brief Appends the next token to \p State and updates information /// necessary for indentation. /// Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -1313,6 +1313,40 @@ if (Current.UnbreakableTailLength >= ColumnLimit) return 0; + // Verify if the comment should be reflown + LineState PrevState = State; + unsigned Penalty = reflowProtrudingToken(Current, State, Token, ColumnLimit, true, true); + bool Reflow = true; + if (Penalty > 0) { +LineState NoReflowState = PrevState; +unsigned NoReflowPenalty = reflowProtrudingToken(Current, NoReflowState, Token, ColumnLimit, + true, false); +if (NoReflowPenalty <= Penalty) { + Reflow = false; + State = NoReflowState; + Penalty = NoReflowPenalty; +} + } + + // Actually do the reflow, if DryRun=false + if (!DryRun) +reflowProtrudingToken(Current, PrevState, Token, ColumnLimit, false, Reflow); + + // Do not count the penalty twice, it will be
[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.
This revision was automatically updated to reflect the committed changes. Closed by commit rL313628: [Sema] Disallow assigning record lvalues with nested const-qualified fields. (authored by bjope). Changed prior to commit: https://reviews.llvm.org/D37254?vs=114340&id=115831#toc Repository: rL LLVM https://reviews.llvm.org/D37254 Files: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/ExprClassification.cpp cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/assign.c Index: cfe/trunk/include/clang/AST/Type.h === --- cfe/trunk/include/clang/AST/Type.h +++ cfe/trunk/include/clang/AST/Type.h @@ -3791,10 +3791,9 @@ return reinterpret_cast(TagType::getDecl()); } - // FIXME: This predicate is a helper to QualType/Type. It needs to - // recursively check all fields for const-ness. If any field is declared - // const, it needs to return false. - bool hasConstFields() const { return false; } + /// Recursively check all fields in the record for const-ness. If any field + /// is declared const, return true. Otherwise, return false. + bool hasConstFields() const; bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } Index: cfe/trunk/include/clang/AST/Expr.h === --- cfe/trunk/include/clang/AST/Expr.h +++ cfe/trunk/include/clang/AST/Expr.h @@ -275,6 +275,7 @@ MLV_LValueCast, // Specialized form of MLV_InvalidExpression. MLV_IncompleteType, MLV_ConstQualified, +MLV_ConstQualifiedField, MLV_ConstAddrSpace, MLV_ArrayType, MLV_NoSetterProperty, @@ -324,6 +325,7 @@ CM_LValueCast, // Same as CM_RValue, but indicates GCC cast-as-lvalue ext CM_NoSetterProperty,// Implicit assignment to ObjC property without setter CM_ConstQualified, + CM_ConstQualifiedField, CM_ConstAddrSpace, CM_ArrayType, CM_IncompleteType Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -5888,14 +5888,17 @@ "cannot assign to %select{non-|}1static data member %2 " "with const-qualified type %3|" "cannot assign to non-static data member within const member function %1|" + "cannot assign to %select{variable %2|non-static data member %2|lvalue}1 " + "with %select{|nested }3const-qualified data member %4|" "read-only variable is not assignable}0">; def note_typecheck_assign_const : Note< "%select{" "function %1 which returns const-qualified type %2 declared here|" "variable %1 declared const here|" "%select{non-|}1static data member %2 declared const here|" - "member function %q1 is declared const here}0">; + "member function %q1 is declared const here|" + "%select{|nested }1data member %2 declared const here}0">; def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, Index: cfe/trunk/test/Sema/assign.c === --- cfe/trunk/test/Sema/assign.c +++ cfe/trunk/test/Sema/assign.c @@ -16,3 +16,46 @@ b[4] = 1; // expected-error {{read-only variable is not assignable}} b2[4] = 1; // expected-error {{read-only variable is not assignable}} } + +typedef struct I { + const int a; // expected-note 4{{nested data member 'a' declared const here}} \ + expected-note 6{{data member 'a' declared const here}} +} I; +typedef struct J { + struct I i; +} J; +typedef struct K { + struct J *j; +} K; + +void testI(struct I i1, struct I i2) { + i1 = i2; // expected-error {{cannot assign to variable 'i1' with const-qualified data member 'a'}} +} +void testJ1(struct J j1, struct J j2) { + j1 = j2; // expected-error {{cannot assign to variable 'j1' with nested const-qualified data member 'a'}} +} +void testJ2(struct J j, struct I i) { + j.i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}} +} +void testK1(struct K k, struct J j) { + *(k.j) = j; // expected-error {{cannot assign to lvalue with nested const-qualified data member 'a'}} +} +void testK2(struct K k, struct I i) { + k.j->i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}} +} + +void testI_(I i1, I i2) { + i1 = i2; // expected-error {{cannot assign to variable 'i1' with const-qualified data member 'a'}} +} +void testJ1_(J j1, J j2) { + j1 = j2; // expected-error {{cannot assign to variable 'j1' with nested const-qualified data member 'a'}} +} +void testJ2_(J j, I i) { + j.i = i; // expected-error {{cannot assign to non-static data m
r313628 - [Sema] Disallow assigning record lvalues with nested const-qualified fields.
Author: bjope Date: Tue Sep 19 06:10:30 2017 New Revision: 313628 URL: http://llvm.org/viewvc/llvm-project?rev=313628&view=rev Log: [Sema] Disallow assigning record lvalues with nested const-qualified fields. Summary: According to C99 6.3.2.1p1, structs and unions with nested const-qualified fields (that is, const-qualified fields declared at some recursive level of the aggregate) are not modifiable lvalues. However, Clang permits assignments of these lvalues. With this patch, we both prohibit the assignment of records with const-qualified fields and emit a best-effort diagnostic. This fixes https://bugs.llvm.org/show_bug.cgi?id=31796 . Committing on behalf of bevinh (Bevin Hansson). Reviewers: rtrieu, rsmith, bjope Reviewed By: bjope Subscribers: Ka-Ka, rogfer01, bjope, fhahn, cfe-commits Differential Revision: https://reviews.llvm.org/D37254 Modified: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/ExprClassification.cpp cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/assign.c Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=313628&r1=313627&r2=313628&view=diff == --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Tue Sep 19 06:10:30 2017 @@ -275,6 +275,7 @@ public: MLV_LValueCast, // Specialized form of MLV_InvalidExpression. MLV_IncompleteType, MLV_ConstQualified, +MLV_ConstQualifiedField, MLV_ConstAddrSpace, MLV_ArrayType, MLV_NoSetterProperty, @@ -324,6 +325,7 @@ public: CM_LValueCast, // Same as CM_RValue, but indicates GCC cast-as-lvalue ext CM_NoSetterProperty,// Implicit assignment to ObjC property without setter CM_ConstQualified, + CM_ConstQualifiedField, CM_ConstAddrSpace, CM_ArrayType, CM_IncompleteType Modified: cfe/trunk/include/clang/AST/Type.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=313628&r1=313627&r2=313628&view=diff == --- cfe/trunk/include/clang/AST/Type.h (original) +++ cfe/trunk/include/clang/AST/Type.h Tue Sep 19 06:10:30 2017 @@ -3791,10 +3791,9 @@ public: return reinterpret_cast(TagType::getDecl()); } - // FIXME: This predicate is a helper to QualType/Type. It needs to - // recursively check all fields for const-ness. If any field is declared - // const, it needs to return false. - bool hasConstFields() const { return false; } + /// Recursively check all fields in the record for const-ness. If any field + /// is declared const, return true. Otherwise, return false. + bool hasConstFields() const; bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313628&r1=313627&r2=313628&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 06:10:30 2017 @@ -5888,6 +5888,8 @@ def err_typecheck_assign_const : Error< "cannot assign to %select{non-|}1static data member %2 " "with const-qualified type %3|" "cannot assign to non-static data member within const member function %1|" + "cannot assign to %select{variable %2|non-static data member %2|lvalue}1 " + "with %select{|nested }3const-qualified data member %4|" "read-only variable is not assignable}0">; def note_typecheck_assign_const : Note< @@ -5895,7 +5897,8 @@ def note_typecheck_assign_const : Note< "function %1 which returns const-qualified type %2 declared here|" "variable %1 declared const here|" "%select{non-|}1static data member %2 declared const here|" - "member function %q1 is declared const here}0">; + "member function %q1 is declared const here|" + "%select{|nested }1data member %2 declared const here}0">; def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, Modified: cfe/trunk/lib/AST/ExprClassification.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprClassification.cpp?rev=313628&r1=313627&r2=313628&view=diff == --- cfe/trunk/lib/AST/ExprClassification.cpp (original) +++ cfe/trunk/lib/AST/ExprClassification.cpp Tue Sep 19 06:10:30 2017 @@ -641,7 +641,7 @@ static Cl::ModifiableType IsModifiable(A // Records with any const fields (recursively) are not modifiable. if (const RecordType *R = CT->getAs())
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst added inline comments. Comment at: tools/clang-format/clang-format.el:125 If called interactively uses the region or the current statement if there is no active region. If no style is given uses `clang-format-style'." (interactive Please document the ASSUME-FILE parameter. Comment at: tools/clang-format/clang-format.el:187 ;;;###autoload -(defun clang-format-buffer (&optional style) +(defun clang-format-buffer (&optional style assume-file) "Use clang-format to format the current buffer according to STYLE." Nit: consider renaming the parameter to ASSUME-FILENAME so that it's clear that it's a filename. Same above. Comment at: tools/clang-format/clang-format.el:188 +(defun clang-format-buffer (&optional style assume-file) "Use clang-format to format the current buffer according to STYLE." (interactive) Please document the new parameter here as well. https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
klimek added a comment. I find the current semantics of the functions a bit surprising, specifically: ... reflowProtrudingToken(..., bool Reflow) is really confusing me :) I'd have expected something like this where we currently call breakProtrudingToken(): if (CanBreak) { ReflowPenalty = breakProtrudingToken(Current, State, /*DryRun=*/true); FixedPenalty = getExcessPenalty(Current, State); Penalty = ReflowPenalty < FixedPenalty ? ReflowPenalty : FixedPenalty; if (!DryRun && ReflowPenalty < FixedPenalty breakProtrudingToken(Current, State, /*DryRun=*/false); } I haven't looked how we calculate the penalty currently if we can't break, as if we don't, that also seems ... wrong. getExcessPenalty would be a new function that just focuses on getting the excess penalty for a breakable token. https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst accepted this revision. phst added a comment. This revision is now accepted and ready to land. Your use case sounds good to me. Please be sure to document the new parameter, though. Comment at: tools/clang-format/clang-format.el:154 + `("-output-replacements-xml" + ,@(and assume-file (list "-assume-filename" assume-file)) + "-style" ,style Please add a comment referencing the LLVM bug you've just filed so that others aren't surprised about this. https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37955: [libcxx] Fix invert negative bracket match.
mclow.lists requested changes to this revision. mclow.lists added a comment. This revision now requires changes to proceed. When I applied this patch locally, some of the other tests started failing. Specifically: Assertion failed: (!std::regex_match(s, m, std::regex("^[a-f]$", std::regex_constants::basic))), function main, file test/std/re/re.alg/re.alg.match/basic.pass.cpp, line 499. Assertion failed: (!std::regex_match(s, m, std::regex("^[a-f]$", std::regex_constants::extended))), function main, file test/std/re/re.alg/re.alg.match/extended.pass.cpp, line 497. Assertion failed: (!std::regex_match(s, m, std::regex("^[a-f]$"))), function main, file test/std/re/re.alg/re.alg.match/ecma.pass.cpp, line 493. Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$", std::regex_constants::awk))), function main, file test/std/re/re.alg/re.alg.search/awk.pass.cpp, line 560. Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$", std::regex_constants::basic))), function main, file test/std/re/re.alg/re.alg.search/basic.pass.cpp, line 562. Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$"))), function main, file test/std/re/re.alg/re.alg.search/ecma.pass.cpp, line 553. Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$", std::regex_constants::extended))), function main, file test/std/re/re.alg/re.alg.search/extended.pass.cpp, line 560. all now fail for me (Mac OS 10.12) https://reviews.llvm.org/D37955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.
curdeius abandoned this revision. curdeius added a comment. Ok. Nice patch. You can close https://bugs.llvm.org/show_bug.cgi?id=19986 now. https://reviews.llvm.org/D37980 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37861: preserving #pragma clang assume_nonnull in preprocessed output
zibi updated this revision to Diff 115838. zibi added a comment. original + review changes https://reviews.llvm.org/D37861 Files: include/clang/Lex/PPCallbacks.h lib/Frontend/PrintPreprocessedOutput.cpp lib/Lex/Pragma.cpp test/Preprocessor/pragma_assume_nonnull.c Index: test/Preprocessor/pragma_assume_nonnull.c === --- /dev/null +++ test/Preprocessor/pragma_assume_nonnull.c @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -E %s | FileCheck %s + +// CHECK: #pragma clang assume_nonnull begin +#pragma clang assume_nonnull begin + +int bar(int * ip) { return *ip; } + +// CHECK: #pragma clang assume_nonnull end +#pragma clang assume_nonnull end + +int foo(int * _Nonnull ip) { return *ip; } + +int main() { +#if __has_feature(assume_nonnull) + return bar(0) + foo(0); // expected-warning 2 {{null passed to a callee that requires a non-null argument}} +#else + return bar(0) + foo(0); // expected-warning {{null passed to a callee that requires a non-null argument}} +#endif +} Index: lib/Lex/Pragma.cpp === --- lib/Lex/Pragma.cpp +++ lib/Lex/Pragma.cpp @@ -1725,21 +1725,26 @@ // The start location we want after processing this. SourceLocation NewLoc; +PPCallbacks *Callbacks = PP.getPPCallbacks(); if (IsBegin) { // Complain about attempts to re-enter an audit. if (BeginLoc.isValid()) { PP.Diag(Loc, diag::err_pp_double_begin_of_assume_nonnull); PP.Diag(BeginLoc, diag::note_pragma_entered_here); } NewLoc = Loc; + if (Callbacks) +Callbacks->PragmaAssumeNonNullBegin(NewLoc); } else { // Complain about attempts to leave an audit that doesn't exist. if (!BeginLoc.isValid()) { PP.Diag(Loc, diag::err_pp_unmatched_end_of_assume_nonnull); return; } NewLoc = SourceLocation(); + if (Callbacks) +Callbacks->PragmaAssumeNonNullEnd(NewLoc); } PP.setPragmaAssumeNonNullLoc(NewLoc); Index: lib/Frontend/PrintPreprocessedOutput.cpp === --- lib/Frontend/PrintPreprocessedOutput.cpp +++ lib/Frontend/PrintPreprocessedOutput.cpp @@ -143,6 +143,8 @@ ArrayRef Ids) override; void PragmaWarningPush(SourceLocation Loc, int Level) override; void PragmaWarningPop(SourceLocation Loc) override; + void PragmaAssumeNonNullBegin(SourceLocation Loc) override; + void PragmaAssumeNonNullEnd(SourceLocation Loc) override; bool HandleFirstTokOnLine(Token &Tok); @@ -549,6 +551,22 @@ setEmittedDirectiveOnThisLine(); } +void PrintPPOutputPPCallbacks:: +PragmaAssumeNonNullBegin(SourceLocation Loc) { + startNewLineIfNeeded(); + MoveToLine(Loc); + OS << "#pragma clang assume_nonnull begin"; + setEmittedDirectiveOnThisLine(); +} + +void PrintPPOutputPPCallbacks:: +PragmaAssumeNonNullEnd(SourceLocation Loc) { + startNewLineIfNeeded(); + MoveToLine(Loc); + OS << "#pragma clang assume_nonnull end"; + setEmittedDirectiveOnThisLine(); +} + /// HandleFirstTokOnLine - When emitting a preprocessed file in -E mode, this /// is called for the first token on each new line. If this really is the start /// of a new logical line, handle it and return true, otherwise return false. Index: include/clang/Lex/PPCallbacks.h === --- include/clang/Lex/PPCallbacks.h +++ include/clang/Lex/PPCallbacks.h @@ -235,6 +235,14 @@ virtual void PragmaWarningPop(SourceLocation Loc) { } + /// \brief Callback invoked when a \#pragma clang assume_nonnull begin directive + /// is read. + virtual void PragmaAssumeNonNullBegin(SourceLocation Loc) {} + + /// \brief Callback invoked when a \#pragma clang assume_nonnull end directive + /// is read. + virtual void PragmaAssumeNonNullEnd(SourceLocation Loc) {} + /// \brief Called by Preprocessor::HandleMacroExpandedIdentifier when a /// macro invocation is found. virtual void MacroExpands(const Token &MacroNameTok, @@ -443,6 +451,16 @@ Second->PragmaWarningPop(Loc); } + void PragmaAssumeNonNullBegin(SourceLocation Loc) override { +First->PragmaAssumeNonNullBegin(Loc); +Second->PragmaAssumeNonNullBegin(Loc); + } + + void PragmaAssumeNonNullEnd(SourceLocation Loc) override { +First->PragmaAssumeNonNullEnd(Loc); +Second->PragmaAssumeNonNullEnd(Loc); + } + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, SourceRange Range, const MacroArgs *Args) override { First->MacroExpands(MacroNameTok, MD, Range, Args); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37861: preserving #pragma clang assume_nonnull in preprocessed output
zibi added a comment. Please be aware that I don't have the commit permission yet since this is my first patch. I will rely on somebody to push it to the trunk. https://reviews.llvm.org/D37861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc added a comment. Hey @jyknight I heard from @erichkeane that you may want a couple more changes to this patch. Please let me know if you have some changes to recommend. @joerg thought this was OK for submission. Thanks --Melanie https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only
Alexander-Shukaev added a comment. Mark, just wanted to check if the review is still somewhere on your radar. Repository: rL LLVM https://reviews.llvm.org/D15465 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.
curdeius added a comment. I confirm what I observed before: when invoking tests in `unittests/Format/NamespaceEndCommentsFixerTest.cpp`, the `const AnnotatedLine *line` parameter in `getNamespaceToken` gets one big line that includes both `namespace` and `{` (something like `namespace\n{\n...`, whereas in tests invoked from `unittests/Format/FormatTests.cpp`, there is a separate line with `namespace\n` and another one with `{\n`. I haven't looked at what provokes this behavior, but I imagine that there are additional passes in `FormatTests`. https://reviews.llvm.org/D37904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36101: Fix usage of right shift operator in fold expressions
Rakete updated this revision to Diff 115842. Rakete added a project: clang. Rakete added a comment. Added the tests to the existing test file for fold operators that I didn't notice before :) https://reviews.llvm.org/D36101 Files: lib/Parse/ParseExpr.cpp test/Parser/cxx1z-fold-expressions.cpp Index: test/Parser/cxx1z-fold-expressions.cpp === --- test/Parser/cxx1z-fold-expressions.cpp +++ test/Parser/cxx1z-fold-expressions.cpp @@ -43,3 +43,8 @@ (int)(undeclared_junk + ...) + // expected-error {{undeclared}} (int)(a + ...); // expected-error {{does not contain any unexpanded}} } + +// fold-operator can be '>' or '>>'. +template bool greater_than() { return (N > ...); } +template bool right_shift() { return (N >> ...); } + Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -270,7 +270,7 @@ return Level > prec::Unknown && Level != prec::Conditional; } static bool isFoldOperator(tok::TokenKind Kind) { - return isFoldOperator(getBinOpPrecedence(Kind, false, true)); + return isFoldOperator(getBinOpPrecedence(Kind, true, true)); } /// \brief Parse a binary expression that starts with \p LHS and has a Index: test/Parser/cxx1z-fold-expressions.cpp === --- test/Parser/cxx1z-fold-expressions.cpp +++ test/Parser/cxx1z-fold-expressions.cpp @@ -43,3 +43,8 @@ (int)(undeclared_junk + ...) + // expected-error {{undeclared}} (int)(a + ...); // expected-error {{does not contain any unexpanded}} } + +// fold-operator can be '>' or '>>'. +template bool greater_than() { return (N > ...); } +template bool right_shift() { return (N >> ...); } + Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -270,7 +270,7 @@ return Level > prec::Unknown && Level != prec::Conditional; } static bool isFoldOperator(tok::TokenKind Kind) { - return isFoldOperator(getBinOpPrecedence(Kind, false, true)); + return isFoldOperator(getBinOpPrecedence(Kind, true, true)); } /// \brief Parse a binary expression that starts with \p LHS and has a ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings
klimek added a comment. Ok, we still need to fix structured bindings in the UnwrappedLineParser. Unfortunately isCppStructuredBinding requires a "previous token" function, which we don't really have in the UnwrappedLineParser. /me goes thinking more about that part of the problem. That should be largely independent of this patch, though, so LG from my side. https://reviews.llvm.org/D35743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38040: [OpenMP] Add an additional test for D34888
gtbercea created this revision. Test for checking if the mapping is performed correctly. This is a test initially included in Patch https://reviews.llvm.org/D29905 Repository: rL LLVM https://reviews.llvm.org/D38040 Files: test/OpenMP/target_map_codegen.cpp Index: test/OpenMP/target_map_codegen.cpp === --- test/OpenMP/target_map_codegen.cpp +++ test/OpenMP/target_map_codegen.cpp @@ -4840,3 +4840,20 @@ } #endif #endif + +///==/// +// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s + +#ifdef CK30 + +void target_maps_parallel_integer(int a){ + int ParamToKernel = a; +#pragma omp target map(tofrom: ParamToKernel) + { +ParamToKernel += 1; + } +} + +// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 { + +#endif Index: test/OpenMP/target_map_codegen.cpp === --- test/OpenMP/target_map_codegen.cpp +++ test/OpenMP/target_map_codegen.cpp @@ -4840,3 +4840,20 @@ } #endif #endif + +///==/// +// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s + +#ifdef CK30 + +void target_maps_parallel_integer(int a){ + int ParamToKernel = a; +#pragma omp target map(tofrom: ParamToKernel) + { +ParamToKernel += 1; + } +} + +// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 { + +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36101: Fix usage of right shift operator in fold expressions
Rakete updated this revision to Diff 115846. Rakete added a comment. Used the correct return type, even if it doesn't really matter to the compiler. https://reviews.llvm.org/D36101 Files: lib/Parse/ParseExpr.cpp test/Parser/cxx1z-fold-expressions.cpp Index: test/Parser/cxx1z-fold-expressions.cpp === --- test/Parser/cxx1z-fold-expressions.cpp +++ test/Parser/cxx1z-fold-expressions.cpp @@ -43,3 +43,8 @@ (int)(undeclared_junk + ...) + // expected-error {{undeclared}} (int)(a + ...); // expected-error {{does not contain any unexpanded}} } + +// fold-operator can be '>' or '>>'. +template bool greater_than() { return (N > ...); } +template int right_shift() { return (N >> ...); } + Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -270,7 +270,7 @@ return Level > prec::Unknown && Level != prec::Conditional; } static bool isFoldOperator(tok::TokenKind Kind) { - return isFoldOperator(getBinOpPrecedence(Kind, false, true)); + return isFoldOperator(getBinOpPrecedence(Kind, true, true)); } /// \brief Parse a binary expression that starts with \p LHS and has a Index: test/Parser/cxx1z-fold-expressions.cpp === --- test/Parser/cxx1z-fold-expressions.cpp +++ test/Parser/cxx1z-fold-expressions.cpp @@ -43,3 +43,8 @@ (int)(undeclared_junk + ...) + // expected-error {{undeclared}} (int)(a + ...); // expected-error {{does not contain any unexpanded}} } + +// fold-operator can be '>' or '>>'. +template bool greater_than() { return (N > ...); } +template int right_shift() { return (N >> ...); } + Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -270,7 +270,7 @@ return Level > prec::Unknown && Level != prec::Conditional; } static bool isFoldOperator(tok::TokenKind Kind) { - return isFoldOperator(getBinOpPrecedence(Kind, false, true)); + return isFoldOperator(getBinOpPrecedence(Kind, true, true)); } /// \brief Parse a binary expression that starts with \p LHS and has a ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37925: Allow specifying sanitizers in blacklists
vlad.tsyrklevich updated this revision to Diff 115849. vlad.tsyrklevich added a comment. - Refactor to make compile() not virtual again - Refactor a confusing use of ASanMask into individual uses of SanitizerKind::{Address,KernelAddress} - 'Sections' -> 'Section names' https://reviews.llvm.org/D37925 Files: docs/ControlFlowIntegrity.rst docs/SanitizerSpecialCaseList.rst include/clang/Basic/SanitizerBlacklist.h include/clang/Basic/SanitizerSpecialCaseList.h lib/AST/Decl.cpp lib/Basic/CMakeLists.txt lib/Basic/SanitizerBlacklist.cpp lib/Basic/SanitizerSpecialCaseList.cpp lib/Basic/XRayLists.cpp lib/CodeGen/CGClass.cpp lib/CodeGen/CGDeclCXX.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h test/CodeGen/Inputs/sanitizer-special-case-list.sanitized.txt test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized1.txt test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized2.txt test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized3.txt test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized4.txt test/CodeGen/sanitizer-special-case-list.c test/CodeGenCXX/cfi-blacklist.cpp Index: test/CodeGenCXX/cfi-blacklist.cpp === --- test/CodeGenCXX/cfi-blacklist.cpp +++ test/CodeGenCXX/cfi-blacklist.cpp @@ -1,6 +1,18 @@ // RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s -// RUN: echo "type:std::*" > %t.txt -// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s + +// Check that blacklisting cfi and cfi-vcall work correctly +// RUN: echo "[cfi-vcall]" > %t.vcall.txt +// RUN: echo "type:std::*" >> %t.vcall.txt +// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.vcall.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s +// +// RUN: echo "[cfi]" > %t.cfi.txt +// RUN: echo "type:std::*" >> %t.cfi.txt +// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.cfi.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s + +// Check that blacklisting non-vcall modes does not affect vcalls +// RUN: echo "[cfi-icall|cfi-nvcall|cfi-cast-strict|cfi-derived-cast|cfi-unrelated-cast]" > %t.other.txt +// RUN: echo "type:std::*" >> %t.other.txt +// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.other.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s struct S1 { virtual void f(); Index: test/CodeGen/sanitizer-special-case-list.c === --- /dev/null +++ test/CodeGen/sanitizer-special-case-list.c @@ -0,0 +1,26 @@ +// Verify that blacklist sections correctly select sanitizers to apply blacklist entries to. +// +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized1.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized2.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized3.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized4.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED +// +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.sanitized.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=SANITIZED + +unsigned i; + +// SANITIZED: @overflow +// UNSANITIZED: @overflow +unsigned overflow() { + // SANITIZED: call {{.*}}void @__ubsan + // UNSANITIZED-NOT: call {{.*}}void @__ubsan + return i * 37; +} + +// SANITIZED: @cfi +// UNSANITIZED: @cfi +void cfi(void (*fp)()) { + // SANITIZED: llvm.type.test + // UNSANITIZED-NOT: llvm.type.test + fp(); +} Index: test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized4.txt === --- /dev/null +++ test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized4.txt @@ -0,0 +1,4 @@ +[c*] +fun:*cfi* +[u*] +fun:*overflow* Index: test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized3.txt ==
[PATCH] D37804: [OpenCL] Handle address space conversion while setting type alignment
Anastasia added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:957 -return Builder.CreateBitCast(Addr, ConvertType(E->getType())); +return Builder.CreatePointerBitCastOrAddrSpaceCast( +Addr, ConvertType(E->getType())); Anastasia wrote: > yaxunl wrote: > > Better assert that only CK_AddressSpaceConversion allows different addr > > spaces in target type. > It seems for consistency then we would have to assert for both different ASes > in non-`addrspacecast` case and equivalent ASes in the other case. > > As condition becomes complicated then, I could as well split into either > creating `bitcast` or `addrspacecast` explicitly at a cost of one simple > check in the return statement but that would be in release build too. Sam, do you prefer an assert that will check both things or a runtime check that would make sure to build the right IR node here? https://reviews.llvm.org/D37804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37925: Allow specifying sanitizers in blacklists
vlad.tsyrklevich marked an inline comment as done. vlad.tsyrklevich added inline comments. Comment at: docs/SanitizerSpecialCaseList.rst:57 + +Sections are regular expressions written in square brackets that denote which +sanitizer the following entries apply to. For example, ``[address]`` specifies eugenis wrote: > Section names are regular expressions > > also, not really a regular expression, more like a wildcard; I don't know > what's the right term for this They are not just wildcards in the sense that `cfi-vcall|cfi-icall` is more than just wildcard matching. I could add a line that explains how `*` behaves here as well? (similarly to what we have in the entries paragraph) Comment at: lib/CodeGen/CGDeclCXX.cpp:322 + if (getLangOpts().Sanitize.hasOneOf(ASanMask)) +if (!isInSanitizerBlacklist(ASanMask, Fn, Loc)) Fn->addFnAttr(llvm::Attribute::SanitizeAddress); eugenis wrote: > vlad.tsyrklevich wrote: > > This use of ASanMask could also confound address & kernel-address as > > @eugenis pointed out. > It would be more readable to split it in two if-s, IMHO: one for asan, one > for kernel-asan. > Agreed. https://reviews.llvm.org/D37925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38041: [libc++] Separate locale tests that are XFAIL on linux-gnu from others
timshen created this revision. Herald added a subscriber: sanjoy. Herald added a reviewer: EricWF. The tests don't pass on linux-gnu. Move them out, so that the others don't have to be XFAIL. https://reviews.llvm.org/D38041 Files: libcxx/test/std/re/re.alg/re.alg.match/basic.pass.cpp libcxx/test/std/re/re.alg/re.alg.match/basic_locale.pass.cpp libcxx/test/std/re/re.alg/re.alg.match/ecma.pass.cpp libcxx/test/std/re/re.alg/re.alg.match/ecma_locale.pass.cpp libcxx/test/std/re/re.alg/re.alg.match/extended.pass.cpp libcxx/test/std/re/re.alg/re.alg.match/extended_locale.pass.cpp libcxx/test/std/re/re.alg/re.alg.search/awk.pass.cpp libcxx/test/std/re/re.alg/re.alg.search/awk_locale.pass.cpp libcxx/test/std/re/re.alg/re.alg.search/ecma.pass.cpp libcxx/test/std/re/re.alg/re.alg.search/ecma_locale.pass.cpp libcxx/test/std/re/re.alg/re.alg.search/extended.pass.cpp libcxx/test/std/re/re.alg/re.alg.search/extended_locale.pass.cpp Index: libcxx/test/std/re/re.alg/re.alg.search/extended_locale.pass.cpp === --- /dev/null +++ libcxx/test/std/re/re.alg/re.alg.search/extended_locale.pass.cpp @@ -0,0 +1,98 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// REQUIRES: locale.cs_CZ.ISO8859-2 + +// + +// template +// bool +// regex_search(BidirectionalIterator first, BidirectionalIterator last, +// match_results& m, +// const basic_regex& e, +// regex_constants::match_flag_type flags = regex_constants::match_default); + +// TODO: investigation needed +// XFAIL: linux-gnu + +#include +#include +#include "test_macros.h" +#include "test_iterators.h" + +#include "platform_support.h" // locale name macros + +int main() +{ +std::locale::global(std::locale(LOCALE_cs_CZ_ISO8859_2)); +{ +std::cmatch m; +const char s[] = "m"; +assert(std::regex_search(s, m, std::regex("[a[=M=]z]", + std::regex_constants::extended))); +assert(m.size() == 1); +assert(!m.prefix().matched); +assert(m.prefix().first == s); +assert(m.prefix().second == m[0].first); +assert(!m.suffix().matched); +assert(m.suffix().first == m[0].second); +assert(m.suffix().second == m[0].second); +assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s)); +assert(m.position(0) == 0); +assert(m.str(0) == s); +} +{ +std::cmatch m; +const char s[] = "Ch"; +assert(std::regex_search(s, m, std::regex("[a[.ch.]z]", + std::regex_constants::extended | std::regex_constants::icase))); +assert(m.size() == 1); +assert(!m.prefix().matched); +assert(m.prefix().first == s); +assert(m.prefix().second == m[0].first); +assert(!m.suffix().matched); +assert(m.suffix().first == m[0].second); +assert(m.suffix().second == m[0].second); +assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s)); +assert(m.position(0) == 0); +assert(m.str(0) == s); +} +{ +std::wcmatch m; +const wchar_t s[] = L"m"; +assert(std::regex_search(s, m, std::wregex(L"[a[=M=]z]", + std::regex_constants::extended))); +assert(m.size() == 1); +assert(!m.prefix().matched); +assert(m.prefix().first == s); +assert(m.prefix().second == m[0].first); +assert(!m.suffix().matched); +assert(m.suffix().first == m[0].second); +assert(m.suffix().second == m[0].second); +assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s)); +assert(m.position(0) == 0); +assert(m.str(0) == s); +} +{ +std::wcmatch m; +const wchar_t s[] = L"Ch"; +assert(std::regex_search(s, m, std::wregex(L"[a[.ch.]z]", + std::regex_constants::extended | std::regex_constants::icase))); +assert(m.size() == 1); +assert(!m.prefix().matched); +assert(m.prefix().first == s); +assert(m.prefix().second == m[0].first); +assert(!m.suffix().matched); +assert(m.suffix().first == m[0].second); +assert(m.suffix().second == m[0].second); +assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s)); +assert(m.position(0) == 0); +assert(m.str(0) == s); +} +} Index: libcxx/test/std/re/re.alg/re.alg.search/extended.pass.cpp ===
[PATCH] D37955: [libcxx] Fix invert negative bracket match.
timshen updated this revision to Diff 115851. timshen added a comment. Fixed. Those tests were XFAILing on linux-gnu. I also created https://reviews.llvm.org/D38041 to XFAIL only on the failing ones. https://reviews.llvm.org/D37955 Files: libcxx/include/regex libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp === --- /dev/null +++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp @@ -0,0 +1,29 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// + +// template +// bool +// regex_search(BidirectionalIterator first, BidirectionalIterator last, +// match_results& m, +// const basic_regex& e, +// regex_constants::match_flag_type flags = regex_constants::match_default); + +#include +#include +#include "test_macros.h" + +// PR34310 +int main() +{ + assert(std::regex_search("HelloWorld", std::regex("[^\\W]"))); + assert(std::regex_search("_", std::regex("[^\\W]"))); + return 0; +} Index: libcxx/include/regex === --- libcxx/include/regex +++ libcxx/include/regex @@ -2409,17 +2409,28 @@ goto __exit; } } -if (!__neg_chars_.empty()) +// set of "__found" chars = +// union(complement(union(__neg_chars_, __neg_mask_)), +// other cases...) +// +// __neg_chars_ and __neg_mask_'d better be handled together, as there +// are no short circuit opportunities. +// +// In addition, when __neg_mask_/__neg_chars_ is empty, they should be +// treated as all ones/all chars. { -for (size_t __i = 0; __i < __neg_chars_.size(); ++__i) -{ -if (__ch == __neg_chars_[__i]) -goto __is_neg_char; -} + const bool __in_neg_mask = (__neg_mask_ == 0) || + __traits_.isctype(__ch, __neg_mask_); + const bool __in_neg_chars = + __neg_chars_.empty() || + std::find(__neg_chars_.begin(), __neg_chars_.end(), __ch) != + __neg_chars_.end(); + if (!(__in_neg_mask || __in_neg_chars)) + { __found = true; goto __exit; + } } -__is_neg_char: if (!__ranges_.empty()) { string_type __s2 = __collate_ ? @@ -2451,11 +2462,6 @@ __found = true; goto __exit; } -if (__neg_mask_ && !__traits_.isctype(__ch, __neg_mask_)) -{ -__found = true; -goto __exit; -} } else __found = __negate_; // force reject Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp === --- /dev/null +++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp @@ -0,0 +1,29 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// + +// template +// bool +// regex_search(BidirectionalIterator first, BidirectionalIterator last, +// match_results& m, +// const basic_regex& e, +// regex_constants::match_flag_type flags = regex_constants::match_default); + +#include +#include +#include "test_macros.h" + +// PR34310 +int main() +{ + assert(std::regex_search("HelloWorld", std::regex("[^\\W]"))); + assert(std::regex_search("_", std::regex("[^\\W]"))); + return 0; +} Index: libcxx/include/regex === --- libcxx/include/regex +++ libcxx/include/regex @@ -2409,17 +2409,28 @@ goto __exit; } } -if (!__neg_chars_.empty()) +// set of "__found" chars = +// union(complement(union(__neg_chars_, __neg_mask_)), +// other cases...) +// +// __neg_chars_ and __neg_mask_'d better be handled together, as there +// are no short circuit opportunities. +// +// In addition, when __neg_mask_/__neg_chars_ is empty, they should be +// treated as all ones/all chars.
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
aprantl created this revision. This patch fixes a regression introduced by r290392 (https://reviews.llvm.org/D28047 — Make '-disable-llvm-optzns' an alias for '-disable-llvm-passes'.) After r290392, CodeGenOpts.DisableLLVMOpts implicitly disables CodeGenOpts.VerifyModule, because the Verifier also happens to be implemented as an LLVM pass. This new behavior is undesirable because the Verifier is clearly not an optimization. One use-case where this is relevant is when using clang to compile bitcode; we don't want the compiler to crash on invalid input just because -disable-llvm-optzns is used. https://reviews.llvm.org/D38042 Files: lib/CodeGen/BackendUtil.cpp test/CodeGen/verify-debuginfo.ll Index: test/CodeGen/verify-debuginfo.ll === --- /dev/null +++ test/CodeGen/verify-debuginfo.ll @@ -0,0 +1,17 @@ +; REQUIRES: x86-registered-target +; RUN: %clang_cc1 -triple i386-apple-darwin -disable-llvm-optzns -S %s -o - 2>&1 \ +; RUN: | FileCheck %s +; CHECK: invalid global variable ref +; CHECK: warning: ignoring invalid debug info in {{.*}}.ll + +@global = common global i32 0, align 4, !dbg !2 + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!5, !6} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "adrian", emissionKind: FullDebug, globals: !{!3}) +!1 = !DIFile(filename: "broken.c", directory: "/") +!2 = !DIGlobalVariableExpression(var: !3, expr: !DIExpression()) +!3 = !DIGlobalVariable(name: "g", scope: !0, file: !1, line: 1, type: !1, isLocal: false, isDefinition: true) +!5 = !{i32 2, !"Dwarf Version", i32 4} +!6 = !{i32 1, !"Debug Info Version", i32 3} Index: lib/CodeGen/BackendUtil.cpp === --- lib/CodeGen/BackendUtil.cpp +++ lib/CodeGen/BackendUtil.cpp @@ -461,8 +461,16 @@ legacy::FunctionPassManager &FPM) { // Handle disabling of all LLVM passes, where we want to preserve the // internal module before any optimization. - if (CodeGenOpts.DisableLLVMPasses) + if (CodeGenOpts.DisableLLVMPasses) { +if (CodeGenOpts.VerifyModule) { + ModulePassManager MPM; + MPM.addPass(VerifierPass(false)); + ModuleAnalysisManager MAM; + MAM.registerPass([&] { return VerifierAnalysis(); }); + MPM.run(*TheModule, MAM); +} return; + } // Figure out TargetLibraryInfo. This needs to be added to MPM and FPM // manually (and not via PMBuilder), since some passes (eg. InstrProfiling) Index: test/CodeGen/verify-debuginfo.ll === --- /dev/null +++ test/CodeGen/verify-debuginfo.ll @@ -0,0 +1,17 @@ +; REQUIRES: x86-registered-target +; RUN: %clang_cc1 -triple i386-apple-darwin -disable-llvm-optzns -S %s -o - 2>&1 \ +; RUN: | FileCheck %s +; CHECK: invalid global variable ref +; CHECK: warning: ignoring invalid debug info in {{.*}}.ll + +@global = common global i32 0, align 4, !dbg !2 + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!5, !6} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "adrian", emissionKind: FullDebug, globals: !{!3}) +!1 = !DIFile(filename: "broken.c", directory: "/") +!2 = !DIGlobalVariableExpression(var: !3, expr: !DIExpression()) +!3 = !DIGlobalVariable(name: "g", scope: !0, file: !1, line: 1, type: !1, isLocal: false, isDefinition: true) +!5 = !{i32 2, !"Dwarf Version", i32 4} +!6 = !{i32 1, !"Debug Info Version", i32 3} Index: lib/CodeGen/BackendUtil.cpp === --- lib/CodeGen/BackendUtil.cpp +++ lib/CodeGen/BackendUtil.cpp @@ -461,8 +461,16 @@ legacy::FunctionPassManager &FPM) { // Handle disabling of all LLVM passes, where we want to preserve the // internal module before any optimization. - if (CodeGenOpts.DisableLLVMPasses) + if (CodeGenOpts.DisableLLVMPasses) { +if (CodeGenOpts.VerifyModule) { + ModulePassManager MPM; + MPM.addPass(VerifierPass(false)); + ModuleAnalysisManager MAM; + MAM.registerPass([&] { return VerifierAnalysis(); }); + MPM.run(*TheModule, MAM); +} return; + } // Figure out TargetLibraryInfo. This needs to be added to MPM and FPM // manually (and not via PMBuilder), since some passes (eg. InstrProfiling) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37913: [OpenMP] Enable the existing nocudalib flag for OpenMP offloading toolchain.
tra added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:255-257 CudaArch gpu_arch = StringToCudaArch(GPUArchName); - assert(gpu_arch != CudaArch::UNKNOWN && + assert((gpu_arch != CudaArch::UNKNOWN || + Args.hasArg(options::OPT_nocudalib)) && The purpose of the original assert was to catch a programming error and this change negates that purpose. Perhaps I'm missing something. Could you elaborate on what's the motivation for this particular change? I don't understand why it would be OK to end up with an unknown GPU architecture if -nocudalib is specified. You still do want to pass *some* specific GPU arch to ptxas and that has nothing to do with whether you happen to have suitable libdevice. Repository: rL LLVM https://reviews.llvm.org/D37913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r313643 - Resubmit "Fix llvm-lit script generation in libcxx."
Author: zturner Date: Tue Sep 19 10:19:10 2017 New Revision: 313643 URL: http://llvm.org/viewvc/llvm-project?rev=313643&view=rev Log: Resubmit "Fix llvm-lit script generation in libcxx." After speaking with the libcxx owners, they agreed that this is a bug in the bot that needs to be fixed by the bot owners, and the CMake changes are correct. Modified: libcxx/trunk/CMakeLists.txt libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake libcxx/trunk/test/CMakeLists.txt Modified: libcxx/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=313643&r1=313642&r2=313643&view=diff == --- libcxx/trunk/CMakeLists.txt (original) +++ libcxx/trunk/CMakeLists.txt Tue Sep 19 10:19:10 2017 @@ -653,6 +653,7 @@ endif() # # However, since some submission systems strip test/ subdirectories, check for # it before adding it. + if(IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test") add_subdirectory(test) endif() @@ -660,6 +661,15 @@ if (LIBCXX_INCLUDE_TESTS) add_subdirectory(lib/abi) endif() +if (LIBCXX_STANDALONE_BUILD AND EXISTS "${LLVM_MAIN_SRC_DIR}/utils/llvm-lit") + # Make sure the llvm-lit script is generated into the bin directory, and do + # it after adding all tests, since the generated script will only work + # correctly discovered tests against test locations from the source tree that + # have already been discovered. + add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit + ${CMAKE_CURRENT_BINARY_DIR}/llvm-lit) +endif() + if (LIBCXX_INCLUDE_DOCS) add_subdirectory(docs) endif() Modified: libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake?rev=313643&r1=313642&r2=313643&view=diff == --- libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake (original) +++ libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake Tue Sep 19 10:19:10 2017 @@ -106,6 +106,11 @@ macro(configure_out_of_tree_llvm) set(LLVM_ENABLE_SPHINX OFF) endif() + # In a standalone build, we don't have llvm to automatically generate the + # llvm-lit script for us. So we need to provide an explicit directory that + # the configurator should write the script into. + set(LLVM_LIT_OUTPUT_DIR "${libcxx_BINARY_DIR}/bin") + # Required LIT Configuration # Define the default arguments to use with 'lit', and an option for the user # to override. Modified: libcxx/trunk/test/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/CMakeLists.txt?rev=313643&r1=313642&r2=313643&view=diff == --- libcxx/trunk/test/CMakeLists.txt (original) +++ libcxx/trunk/test/CMakeLists.txt Tue Sep 19 10:19:10 2017 @@ -49,10 +49,9 @@ set(LIBCXX_EXECUTOR "None" CACHE STRING set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not edit!") -configure_file( +configure_lit_site_cfg( ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in - ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg - @ONLY) + ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg) set(LIBCXX_TEST_DEPS "") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument
ahatanak added a comment. ping https://reviews.llvm.org/D36915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37912: [OpenMP] Bugfix: output file name drops the absolute path where full path is needed.
tra added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:441 -SmallString<256> Name = llvm::sys::path::filename(II.getFilename()); +SmallString<256> Name = StringRef(II.getFilename()); llvm::sys::path::replace_extension(Name, "cubin"); I don't think explicit StringRef is needed here. `SmallString<256> Name(II.getFilename());` should do the job. Repository: rL LLVM https://reviews.llvm.org/D37912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
chandlerc added a comment. But, the verifier itself will just "crash". It won't print a stack trace, but I don't see why that's much better? And this flag is supposed to be a developer option and not a user facing one, so I'm somewhat confused at what the intent is here... https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
lebedev.ri added a comment. Ping, would be really nice to get this going :) Would unblock me for working on the second half of the fix for https://bugs.llvm.org/show_bug.cgi?id=34147 Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38040: [OpenMP] Add an additional test for D34888
tra added a comment. LGTM in general. Comment at: test/OpenMP/target_map_codegen.cpp:4845 +///==/// +// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s + Please break the line to make it easier to read. Repository: rL LLVM https://reviews.llvm.org/D38040 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
aprantl added a comment. In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > But, the verifier itself will just "crash". It won't print a stack trace, but > I don't see why that's much better? And this flag is supposed to be a > developer option and not a user facing one, so I'm somewhat confused at what > the intent is here... No, it will report_fatal_error() instead of crashing the compiler later on. In any case, that is not my primary motivation: The intent is exactly what is illustrated by the testcase, stripping malformed debug info metadata produced by older, buggy versions of clang. The backstory to this is that historically the Verifier was very weak when it came to verifying debug info. To allow us to make the Verifier better (stricter), while still supporting importing of older .bc files produced by older compilers, we added a mechanism that strips all debug info metadata if the verification of the debug info failed, but the bitcode is otherwise correct. https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
chandlerc added a comment. In https://reviews.llvm.org/D38042#875328, @aprantl wrote: > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > > > But, the verifier itself will just "crash". It won't print a stack trace, > > but I don't see why that's much better? And this flag is supposed to be a > > developer option and not a user facing one, so I'm somewhat confused at > > what the intent is here... > > > No, it will report_fatal_error() instead of crashing the compiler later on. > In any case, that is not my primary motivation: The intent is exactly what > is illustrated by the testcase, stripping malformed debug info metadata > produced by older, buggy versions of clang. The backstory to this is that > historically the Verifier was very weak when it came to verifying debug info. > To allow us to make the Verifier better (stricter), while still supporting > importing of older .bc files produced by older compilers, we added a > mechanism that strips all debug info metadata if the verification of the > debug info failed, but the bitcode is otherwise correct. Ok, that use case makes way more sense. I'd replace the change description with some discussion of this use case. My next question is -- why is this done by the verifier? It seems *really* bad that the verifier *changes the IR*. Don't get me wrong, what you're trying to do (strip malformed debug info) makes perfect sense. But I think that the thing which does that shouldn't be called a verifier. It might *use* the verifier of course. Last but not least, I still suspect that this shouldn't be run here. If the user wants to disable LLVM passes *and emits LLVM IR*, they should get it unperturbed. The stripping of malformed debug info seems like it should happen later as part of the passes to emit code, and I'd actually expect the LLVM code generator to add the necessary pass rather than relying on every frontend remembering to do so? https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r313653 - Fix ClangDiagnosticHandler::is*RemarkEnabled members
Author: anemet Date: Tue Sep 19 10:59:40 2017 New Revision: 313653 URL: http://llvm.org/viewvc/llvm-project?rev=313653&view=rev Log: Fix ClangDiagnosticHandler::is*RemarkEnabled members Apparently these weren't really working. I added test coverage and fixed the typo in the name and the parameter. Added: cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=313653&r1=313652&r2=313653&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Tue Sep 19 10:59:40 2017 @@ -53,19 +53,20 @@ namespace clang { : CodeGenOpts(CGOpts), BackendCon(BCon) {} bool handleDiagnostics(const DiagnosticInfo &DI) override; -bool isAnalysisRemarkEnable(const std::string &PassName) { + +bool isAnalysisRemarkEnabled(StringRef PassName) const override { return (CodeGenOpts.OptimizationRemarkAnalysisPattern && CodeGenOpts.OptimizationRemarkAnalysisPattern->match(PassName)); } -bool isMissedOptRemarkEnable(const std::string &PassName) { +bool isMissedOptRemarkEnabled(StringRef PassName) const override { return (CodeGenOpts.OptimizationRemarkMissedPattern && CodeGenOpts.OptimizationRemarkMissedPattern->match(PassName)); } -bool isPassedOptRemarkEnable(const std::string &PassName) { +bool isPassedOptRemarkEnabled(StringRef PassName) const override { return (CodeGenOpts.OptimizationRemarkPattern && CodeGenOpts.OptimizationRemarkPattern->match(PassName)); } - + private: const CodeGenOptions &CodeGenOpts; BackendConsumer *BackendCon; Added: cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c?rev=313653&view=auto == --- cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c (added) +++ cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c Tue Sep 19 10:59:40 2017 @@ -0,0 +1,11 @@ +// Test that the is*RemarkEnabled overrides are working properly. This remark +// requiring extra analysis is only conditionally enabled. + +// RUN: %clang_cc1 %s -Rpass-missed=gvn -O2 -emit-llvm-only -verify + +int foo(int *x, int *y) { + int a = *x; + *y = 2; + // expected-remark@+1 {{load of type i32 not eliminated}} + return a + *x; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
NoQ added a comment. The overall idea makes sense to me. I'd like you to join the effort with Peter who during his work on loop widening came up with a matcher-based procedure for finding out if a variable is changed anywhere; it currently lives in `LoopUnrolling.cpp` and we need only once implementation of that. Repository: rL LLVM https://reviews.llvm.org/D37897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8593 - bool Match = true; + // is this a tautological comparison? if yes, than contains the always-result + llvm::Optional Result; Comments should be complete sentences with capitalization and punctuation (here and elsewhere in the patch). Also, I can't really understand what this comment is saying. Comment at: lib/Sema/SemaChecking.cpp:8596 + Expr *Value; // which one is the value, and not a constant? + const char *cmp; // the comparison used, as string Should be `Cmp` per coding standards. Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
aprantl added a comment. In https://reviews.llvm.org/D38042#875334, @chandlerc wrote: > In https://reviews.llvm.org/D38042#875328, @aprantl wrote: > > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > > > > > But, the verifier itself will just "crash". It won't print a stack trace, > > > but I don't see why that's much better? And this flag is supposed to be a > > > developer option and not a user facing one, so I'm somewhat confused at > > > what the intent is here... > > > > > > No, it will report_fatal_error() instead of crashing the compiler later on. > > In any case, that is not my primary motivation: The intent is exactly what > > is illustrated by the testcase, stripping malformed debug info metadata > > produced by older, buggy versions of clang. The backstory to this is that > > historically the Verifier was very weak when it came to verifying debug > > info. To allow us to make the Verifier better (stricter), while still > > supporting importing of older .bc files produced by older compilers, we > > added a mechanism that strips all debug info metadata if the verification > > of the debug info failed, but the bitcode is otherwise correct. > > > Ok, that use case makes way more sense. I'd replace the change description > with some discussion of this use case. > > My next question is -- why is this done by the verifier? It seems *really* > bad that the verifier *changes the IR*. Don't get me wrong, what you're > trying to do (strip malformed debug info) makes perfect sense. But I think > that the thing which does that shouldn't be called a verifier. It might *use* > the verifier of course. That was a purely pragmatic decision: Most (but not all) LLVM-based tools are running the Verifier as an LLVM pass so adding the stripping into the pass was the least invasive way of implementing this feature. If we are truly bothered by this, I think what could work is to separate out a second StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to run immediately after it. I don't see this adding much value though, and we would have to modify all tools to schedule the new pass explicitly. Do you think that would be worth pursuing? > Last but not least, I still suspect that this shouldn't be run here. If the > user wants to disable LLVM passes *and emits LLVM IR*, they should get it > unperturbed. The stripping of malformed debug info seems like it should > happen later as part of the passes to emit code, and I'd actually expect the > LLVM code generator to add the necessary pass rather than relying on every > frontend remembering to do so? The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not LLVM passes. Also, I believe the Verifier is special. The Verifier protects LLVM from crashing and as a user I think I would prefer a Verifier error over clang crashing while emitting bitcode. Because of auto-upgrades users already don't get the IR unperturbed, and I view stripping of broken debug info as a (in all fairness: very brutal :-) auto-upgrade. https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
chandlerc added a comment. In https://reviews.llvm.org/D38042#875412, @aprantl wrote: > In https://reviews.llvm.org/D38042#875334, @chandlerc wrote: > > > In https://reviews.llvm.org/D38042#875328, @aprantl wrote: > > > > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > > > > > > > But, the verifier itself will just "crash". It won't print a stack > > > > trace, but I don't see why that's much better? And this flag is > > > > supposed to be a developer option and not a user facing one, so I'm > > > > somewhat confused at what the intent is here... > > > > > > > > > No, it will report_fatal_error() instead of crashing the compiler later > > > on. > > > In any case, that is not my primary motivation: The intent is exactly > > > what is illustrated by the testcase, stripping malformed debug info > > > metadata produced by older, buggy versions of clang. The backstory to > > > this is that historically the Verifier was very weak when it came to > > > verifying debug info. To allow us to make the Verifier better (stricter), > > > while still supporting importing of older .bc files produced by older > > > compilers, we added a mechanism that strips all debug info metadata if > > > the verification of the debug info failed, but the bitcode is otherwise > > > correct. > > > > > > Ok, that use case makes way more sense. I'd replace the change description > > with some discussion of this use case. > > > > My next question is -- why is this done by the verifier? It seems *really* > > bad that the verifier *changes the IR*. Don't get me wrong, what you're > > trying to do (strip malformed debug info) makes perfect sense. But I think > > that the thing which does that shouldn't be called a verifier. It might > > *use* the verifier of course. > > > That was a purely pragmatic decision: Most (but not all) LLVM-based tools are > running the Verifier as an LLVM pass so adding the stripping into the pass > was the least invasive way of implementing this feature. If we are truly > bothered by this, I think what could work is to separate out a second > StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to > run immediately after it. I don't see this adding much value though, and we > would have to modify all tools to schedule the new pass explicitly. Do you > think that would be worth pursuing? Absolutely. I think the verifier should never, under any circumstances, mutate the IR. Think about it, with the current design if a pass corrupts the debug info the verifier may "hide" this by stripping it out rather than allowing us to find it. > > >> Last but not least, I still suspect that this shouldn't be run here. If the >> user wants to disable LLVM passes *and emits LLVM IR*, they should get it >> unperturbed. The stripping of malformed debug info seems like it should >> happen later as part of the passes to emit code, and I'd actually expect the >> LLVM code generator to add the necessary pass rather than relying on every >> frontend remembering to do so? > > The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not > LLVM passes. (sorry for the off-list duplication, but it belongs here anyways) I disagree. `-disable-llvm-optzns` is a developer flag, and was almost an alias for `-disable-llvm-passes`. After discussion on the list we made it an actual legacy and deprecated alias for `-disable-llvm-passes` because the latter was more clear, understandable, and correct. We had cases where the passes run by `-disable-llvm-optzns` were actually problematic and we wanted to debug them but were unable to do so. > Also, I believe the Verifier is special. The Verifier protects LLVM from > crashing and as a user I think I would prefer a Verifier error over clang > crashing while emitting bitcode. I think this distinction is not a meaningful one ultimately. And the verifier should *never* be a functional requirement because it should have no effect. I'm happy for us to verify when we read input, but even then it should not mutate the IR. > Because of auto-upgrades users already don't get the IR unperturbed, and I > view stripping of broken debug info as a (in all fairness: very brutal :-) > auto-upgrade. But auto-upgrade happens on *read*. If you want to add the debug info stripping to auto-upgrade, that's a reasonable discussion to have, and no change here will be required. There might be concerns about that on the LLVM side, I don't know. But the verifier (as well as running it here) does not seem like the right solution. https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38048: [clangd] Add textDocument/signatureHelp
rwols created this revision. Makes clangd respond to a client's "textDocument/signatureHelp" request by presenting function/method overloads. Still need to add tests. Also, there is duplicate code in clangd::codeComplete and clangd::signatureHelp now, maybe refactor this to a common function? https://reviews.llvm.org/D38048 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/Protocol.cpp clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h Index: clangd/ProtocolHandlers.h === --- clangd/ProtocolHandlers.h +++ clangd/ProtocolHandlers.h @@ -46,6 +46,8 @@ JSONOutput &Out) = 0; virtual void onCompletion(TextDocumentPositionParams Params, StringRef ID, JSONOutput &Out) = 0; + virtual void onSignatureHelp(TextDocumentPositionParams Params, StringRef ID, + JSONOutput &Out) = 0; virtual void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID, JSONOutput &Out) = 0; }; Index: clangd/ProtocolHandlers.cpp === --- clangd/ProtocolHandlers.cpp +++ clangd/ProtocolHandlers.cpp @@ -186,6 +186,23 @@ ProtocolCallbacks &Callbacks; }; +struct SignatureHelpHandler : Handler { + SignatureHelpHandler(JSONOutput &Output, ProtocolCallbacks &Callbacks) + : Handler(Output), Callbacks(Callbacks) {} + + void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override { +auto TDPP = TextDocumentPositionParams::parse(Params); +if (!TDPP) { + Output.log("Failed to decode TextDocumentPositionParams!\n"); + return; +} +Callbacks.onSignatureHelp(*TDPP, ID, Output); + } + +private: + ProtocolCallbacks &Callbacks; +}; + struct GotoDefinitionHandler : Handler { GotoDefinitionHandler(JSONOutput &Output, ProtocolCallbacks &Callbacks) : Handler(Output), Callbacks(Callbacks) {} @@ -238,6 +255,9 @@ "textDocument/completion", llvm::make_unique(Out, Callbacks)); Dispatcher.registerHandler( + "textDocument/signatureHelp", + llvm::make_unique(Out, Callbacks)); + Dispatcher.registerHandler( "textDocument/definition", llvm::make_unique(Out, Callbacks)); } Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -400,6 +400,71 @@ static std::string unparse(const CompletionItem &P); }; +/// Represents a parameter of a callable-signature. +/// +/// A parameter can have a label and a doc-comment. +struct ParameterInformation { + + /// The label of this parameter. Will be shown in the UI. + std::string label; + + /// The human-readable doc-comment of this parameter. Will be shown in the UI + /// but can be omitted. + std::string documentation; + + static std::string unparse(const ParameterInformation &); +}; + +/// Represents the signature of something callable. +/// +/// A signature can have a label, like a function-name, a doc-comment, and a set +/// of parameters. +struct SignatureInformation { + + /// The label of this signature. Will be shown in the UI. + std::string label; + + /// The human-readable doc-comment of this signature. Will be shown in the UI + /// but can be omitted. + std::string documentation; + + /// The parameters of this signature. + std::vector parameters; + + static std::string unparse(const SignatureInformation &); +}; + +/// Signature help represents the signature of something callable. +/// +/// There can be multiple signature but only one active and only one active +/// parameter. +struct SignatureHelp { + + /// One or more signatures. + std::vector signatures; + + /// The active signature. + /// + /// If omitted or the value lies outside the range of `signatures` the value + /// defaults to zero or is ignored if `signatures.length === 0`. Whenever + /// possible implementors should make an active decision about the active + /// signature and shouldn't rely on a default value. In future version of the + /// protocol this property might become mandantory to better express this. + int activeSignature = 0; + + /// The active parameter of the active signature. + /// + /// If omitted or the value lies outside the range of + /// `signatures[activeSignature].parameters` defaults to 0 if the active + /// signature has parameters. If the active signature has no parameters it is + /// ignored. In future version of the protocol this property might become + /// mandantory to better express the active parameter if the active signature + /// does have any. + int activeParameter = 0; + + static std::string unparse(const SignatureHelp &); +}; + } // namespace clangd } // namespace clang Index: clangd/Protocol.cpp ==
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
aprantl added a comment. In https://reviews.llvm.org/D38042#875412, @aprantl wrote: > In https://reviews.llvm.org/D38042#875334, @chandlerc wrote: > > > In https://reviews.llvm.org/D38042#875328, @aprantl wrote: > > > > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > > > > > > > But, the verifier itself will just "crash". It won't print a stack > > > > trace, but I don't see why that's much better? And this flag is > > > > supposed to be a developer option and not a user facing one, so I'm > > > > somewhat confused at what the intent is here... > > > > > > > > > No, it will report_fatal_error() instead of crashing the compiler later > > > on. > > > In any case, that is not my primary motivation: The intent is exactly > > > what is illustrated by the testcase, stripping malformed debug info > > > metadata produced by older, buggy versions of clang. The backstory to > > > this is that historically the Verifier was very weak when it came to > > > verifying debug info. To allow us to make the Verifier better (stricter), > > > while still supporting importing of older .bc files produced by older > > > compilers, we added a mechanism that strips all debug info metadata if > > > the verification of the debug info failed, but the bitcode is otherwise > > > correct. > > > > > > Ok, that use case makes way more sense. I'd replace the change description > > with some discussion of this use case. > > > > My next question is -- why is this done by the verifier? It seems *really* > > bad that the verifier *changes the IR*. Don't get me wrong, what you're > > trying to do (strip malformed debug info) makes perfect sense. But I think > > that the thing which does that shouldn't be called a verifier. It might > > *use* the verifier of course. > > > That was a purely pragmatic decision: Most (but not all) LLVM-based tools are > running the Verifier as an LLVM pass so adding the stripping into the pass > was the least invasive way of implementing this feature. If we are truly > bothered by this, I think what could work is to separate out a second > StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to > run immediately after it. I don't see this adding much value though, and we > would have to modify all tools to schedule the new pass explicitly. Do you > think that would be worth pursuing? > > > Last but not least, I still suspect that this shouldn't be run here. If the > > user wants to disable LLVM passes *and emits LLVM IR*, they should get it > > unperturbed. The stripping of malformed debug info seems like it should > > happen later as part of the passes to emit code, and I'd actually expect > > the LLVM code generator to add the necessary pass rather than relying on > > every frontend remembering to do so? > > The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not > LLVM passes. It was just pointed out that `-disable-llvm-optzns` is now a deprecated alias for `-disable-llvm-passes`, so I see that this argument makes less sense today. Do you think that `-disable-llvm-passes` should imply `-disable-llvm-verifier`? Is it obvious to users the the Verifier is a pass? If yes, how about adding a positive version of `-disable-llvm-verifier`? > Also, I believe the Verifier is special. The Verifier protects LLVM from > crashing and as a user I think I would prefer a Verifier error over clang > crashing while emitting bitcode. Because of auto-upgrades users already don't > get the IR unperturbed, and I view stripping of broken debug info as a (in > all fairness: very brutal :-) auto-upgrade. https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 115875. werbitt edited the summary of this revision. werbitt added a comment. Rename assume-file to assume-file-name and update documentation https://reviews.llvm.org/D37903 Files: tools/clang-format/clang-format.el Index: tools/clang-format/clang-format.el === --- tools/clang-format/clang-format.el +++ tools/clang-format/clang-format.el @@ -119,20 +119,22 @@ (byte-to-position (1+ byte) ;;;###autoload -(defun clang-format-region (start end &optional style assume-file) - "Use clang-format to format the code between START and END according to STYLE. -If called interactively uses the region or the current statement if there -is no active region. If no style is given uses `clang-format-style'." +(defun clang-format-region (start end &optional style assume-file-name) + "Use clang-format to format the code between START and END according to STYLE +using ASSUME-FILE-NAME to locate a style config file. If called interactively +uses the region or the current statement if there is no active region. If no +style is given uses `clang-format-style'. If no assume-file-name is given uses +`buffer-file-name'." (interactive (if (use-region-p) (list (region-beginning) (region-end)) (list (point) (point (unless style (setq style clang-format-style)) - (unless assume-file -(setq assume-file buffer-file-name)) + (unless assume-file-name +(setq assume-file-name buffer-file-name)) (let ((file-start (clang-format--bufferpos-to-filepos start 'approximate 'utf-8-unix)) @@ -151,7 +153,12 @@ nil nil clang-format-executable nil `(,temp-buffer ,temp-file) nil `("-output-replacements-xml" - ,@(and assume-file (list "-assume-filename" assume-file)) + ;; Gaurd against a nil assume-file-name. + ;; If -assume-filename is given a blank string + ;; it will crash as per the following bug report + ;; https://bugs.llvm.org/show_bug.cgi?id=34667 + ,@(and assume-file-name + (list "-assume-filename" assume-file-name)) "-style" ,style "-offset" ,(number-to-string file-start) "-length" ,(number-to-string (- file-end file-start)) @@ -183,10 +190,12 @@ (when (buffer-name temp-buffer) (kill-buffer temp-buffer) ;;;###autoload -(defun clang-format-buffer (&optional style assume-file) - "Use clang-format to format the current buffer according to STYLE." +(defun clang-format-buffer (&optional style assume-file-name) + "Use clang-format to format the current buffer according to STYLE using +ASSUME-FILE-NAME to locate a style config file. If no style is given uses +`clang-format-style'. If no assume-file-name is given uses `buffer-file-name'." (interactive) - (clang-format-region (point-min) (point-max) style assume-file)) + (clang-format-region (point-min) (point-max) style assume-file-name)) ;;;###autoload (defalias 'clang-format 'clang-format-region) Index: tools/clang-format/clang-format.el === --- tools/clang-format/clang-format.el +++ tools/clang-format/clang-format.el @@ -119,20 +119,22 @@ (byte-to-position (1+ byte) ;;;###autoload -(defun clang-format-region (start end &optional style assume-file) - "Use clang-format to format the code between START and END according to STYLE. -If called interactively uses the region or the current statement if there -is no active region. If no style is given uses `clang-format-style'." +(defun clang-format-region (start end &optional style assume-file-name) + "Use clang-format to format the code between START and END according to STYLE +using ASSUME-FILE-NAME to locate a style config file. If called interactively +uses the region or the current statement if there is no active region. If no +style is given uses `clang-format-style'. If no assume-file-name is given uses +`buffer-file-name'." (interactive (if (use-region-p) (list (region-beginning) (region-end)) (list (point) (point (unless style (setq style clang-format-style)) - (unless assume-file -(setq assume-file buffer-file-name)) + (unless assume-file-name +(setq assume-file-name buffer-file-name)) (let ((file-start (clang-format--bufferpos-to-filepos start 'approximate 'utf-8-unix)) @@ -151,7 +153,12 @@ nil nil clang-format-executable nil `(,temp-buffer ,temp
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
lebedev.ri updated this revision to Diff 115877. lebedev.ri added a comment. 1. Correctly added `TautologicalUnsignedEnumZeroCompare` to `TautologicalCompare` group (sigh) 2. Hopefully fixed spelling of the comments and added a bit better comments 3. Fixed variable names to comply with the coding style. Repository: rL LLVM https://reviews.llvm.org/D37629 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/compare.c test/Sema/tautological-unsigned-enum-zero-compare.c Index: test/Sema/tautological-unsigned-enum-zero-compare.c === --- /dev/null +++ test/Sema/tautological-unsigned-enum-zero-compare.c @@ -0,0 +1,46 @@ +// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s + +int main() { + enum A { A_foo, A_bar }; + enum A a; + +#ifdef TEST + if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} +return 0; + if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} +return 0; + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} +return 0; + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} +return 0; + if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} +return 0; + if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} +return 0; + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} +return 0; + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} +return 0; +#else + // expected-no-diagnostics + if (a < 0) +return 0; + if (a >= 0) +return 0; + if (0 <= a) +return 0; + if (0 > a) +return 0; + if (a < 0U) +return 0; + if (a >= 0U) +return 0; + if (0U <= a) +return 0; + if (0U > a) +return 0; +#endif + + return 1; +} Index: test/Sema/compare.c === --- test/Sema/compare.c +++ test/Sema/compare.c @@ -308,8 +308,59 @@ int rdar8511238() { enum A { A_foo, A_bar }; enum A a; + + if (a == 0) + return 0; + if (a != 0) + return 0; if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} -return 0; + return 0; + if (a <= 0) + return 0; + if (a > 0) + return 0; + if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} + return 0; + + if (0 == a) + return 0; + if (0 != a) + return 0; + if (0 < a) + return 0; + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} + return 0; + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} + return 0; + if (0 >= a) + return 0; + + if (a == 0U) + return 0; + if (a != 0U) + return 0; + if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} + return 0; + if (a <= 0U) + return 0; + if (a > 0U) + return 0; + if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} + return 0; + + if (0U == a) + return 0; + if (0U != a) + return 0; + if (0U < a) + return 0; + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} + return 0; + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} + return 0; + if (0U >= a) + return 0; + return 20; } Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -8583,35 +8583,89 @@ // bool values are handled by DiagnoseOutOfRangeComparison(). - BinaryOperatorKind op = E->getOpcode(); + BinaryOperatorKind Op = E->getOpcode(); if (E->isValueDependent()) return false; Expr *LHS = E->getLHS(); Expr *RHS = E->getRHS(); - bool Match = true; + // Result is used for two purposes. If the Result is empty, then we decided + // that given comparison is non-tautological, and no warning should emitted. + // Alternatively, if the Result is not empty, then it contains the result of + // the comparison. Since the comparison is tautological, it always evaluates + // to the same result - be it either true or false. + llvm::Optional Result; - if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { -S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison) - << "< 0" << "false" << HasEnumType(LHS) - << LHS->getSour
[PATCH] D37413: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Repository: rL LLVM https://reviews.llvm.org/D37413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37466: D37461: fixups for existing InlineAsm tests + adding new ones
rnk added inline comments. Comment at: test/CodeGen/ms-inline-asm.cpp:37-38 - int lvar = 10; - __asm mov eax, offset Foo::ptr - __asm mov eax, offset Foo::Bar::ptr -// CHECK-LABEL: define void @_Z2t2v() These don't seem tested anywhere now Repository: rL LLVM https://reviews.llvm.org/D37466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
aprantl added a comment. In https://reviews.llvm.org/D38042#875418, @chandlerc wrote: > In https://reviews.llvm.org/D38042#875412, @aprantl wrote: > > > In https://reviews.llvm.org/D38042#875334, @chandlerc wrote: > > > > > In https://reviews.llvm.org/D38042#875328, @aprantl wrote: > > > > > > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > > > > > > > > > But, the verifier itself will just "crash". It won't print a stack > > > > > trace, but I don't see why that's much better? And this flag is > > > > > supposed to be a developer option and not a user facing one, so I'm > > > > > somewhat confused at what the intent is here... > > > > > > > > > > > > No, it will report_fatal_error() instead of crashing the compiler later > > > > on. > > > > In any case, that is not my primary motivation: The intent is exactly > > > > what is illustrated by the testcase, stripping malformed debug info > > > > metadata produced by older, buggy versions of clang. The backstory to > > > > this is that historically the Verifier was very weak when it came to > > > > verifying debug info. To allow us to make the Verifier better > > > > (stricter), while still supporting importing of older .bc files > > > > produced by older compilers, we added a mechanism that strips all debug > > > > info metadata if the verification of the debug info failed, but the > > > > bitcode is otherwise correct. > > > > > > > > > Ok, that use case makes way more sense. I'd replace the change > > > description with some discussion of this use case. > > > > > > My next question is -- why is this done by the verifier? It seems > > > *really* bad that the verifier *changes the IR*. Don't get me wrong, what > > > you're trying to do (strip malformed debug info) makes perfect sense. But > > > I think that the thing which does that shouldn't be called a verifier. It > > > might *use* the verifier of course. > > > > > > That was a purely pragmatic decision: Most (but not all) LLVM-based tools > > are running the Verifier as an LLVM pass so adding the stripping into the > > pass was the least invasive way of implementing this feature. If we are > > truly bothered by this, I think what could work is to separate out a second > > StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to > > run immediately after it. I don't see this adding much value though, and we > > would have to modify all tools to schedule the new pass explicitly. Do you > > think that would be worth pursuing? > > > Absolutely. I think the verifier should never, under any circumstances, > mutate the IR. Think about it, with the current design if a pass corrupts the > debug info the verifier may "hide" this by stripping it out rather than > allowing us to find it. Okay, I think I agree. Before I venture off implementing this, do you think that separating out a StripBrokenDebugInfoPass that depends on the Verifier is right way forward? >> >> >>> Last but not least, I still suspect that this shouldn't be run here. If the >>> user wants to disable LLVM passes *and emits LLVM IR*, they should get it >>> unperturbed. The stripping of malformed debug info seems like it should >>> happen later as part of the passes to emit code, and I'd actually expect >>> the LLVM code generator to add the necessary pass rather than relying on >>> every frontend remembering to do so? >> >> The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not >> LLVM passes. > > (sorry for the off-list duplication, but it belongs here anyways) > > I disagree. `-disable-llvm-optzns` is a developer flag, and was almost an > alias for `-disable-llvm-passes`. After discussion on the list we made it an > actual legacy and deprecated alias for `-disable-llvm-passes` because the > latter was more clear, understandable, and correct. We had cases where the > passes run by `-disable-llvm-optzns` were actually problematic and we wanted > to debug them but were unable to do so. > >> Also, I believe the Verifier is special. The Verifier protects LLVM from >> crashing and as a user I think I would prefer a Verifier error over clang >> crashing while emitting bitcode. > > I think this distinction is not a meaningful one ultimately. And the verifier > should *never* be a functional requirement because it should have no effect. > I'm happy for us to verify when we read input, but even then it should not > mutate the IR. > >> Because of auto-upgrades users already don't get the IR unperturbed, and I >> view stripping of broken debug info as a (in all fairness: very brutal :-) >> auto-upgrade. > > But auto-upgrade happens on *read*. If you want to add the debug info > stripping to auto-upgrade, that's a reasonable discussion to have, and no > change here will be required. There might be concerns about that on the LLVM > side, I don't know. But the verifier (as well as running it here) does not > seem like the right solution. Would
[PATCH] D37544: [ubsan] Skip alignment checks which are folded away
vsk updated this revision to Diff 115879. vsk added a comment. - Use a better test case. This one was lifted from an actual example in X86CallingConv.h:86. https://reviews.llvm.org/D37544 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/ubsan-suppress-checks.cpp Index: test/CodeGenCXX/ubsan-suppress-checks.cpp === --- test/CodeGenCXX/ubsan-suppress-checks.cpp +++ test/CodeGenCXX/ubsan-suppress-checks.cpp @@ -17,6 +17,16 @@ // CHECK: ret void } +// CHECK-LABEL: define void @_Z31use_us16_aligned_array_elementsv +void use_us16_aligned_array_elements() { + static const unsigned short Arr[] = {0, 1, 2}; + auto use_array = [](const unsigned short(&X)[3]) -> void {}; + use_array(Arr); + + // CHECK-NOT: and i64 {{.*}}, !nosanitize + // CHECK: ret void +} + struct A { int foo; @@ -229,4 +239,5 @@ d->load_member_3(); load_non_null_pointers(); + use_us16_aligned_array_elements(); } Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -618,6 +618,7 @@ auto PtrToAlloca = dyn_cast(Ptr->stripPointerCastsNoFollowAliases()); + llvm::Value *True = llvm::ConstantInt::getTrue(getLLVMContext()); llvm::Value *IsNonNull = nullptr; bool IsGuaranteedNonNull = SkippedChecks.has(SanitizerKind::Null) || PtrToAlloca; @@ -629,8 +630,7 @@ // The IR builder can constant-fold the null check if the pointer points to // a constant. -IsGuaranteedNonNull = -IsNonNull == llvm::ConstantInt::getTrue(getLLVMContext()); +IsGuaranteedNonNull = IsNonNull == True; // Skip the null check if the pointer is known to be non-null. if (!IsGuaranteedNonNull) { @@ -684,7 +684,8 @@ PtrAsInt, llvm::ConstantInt::get(IntPtrTy, AlignVal - 1)); llvm::Value *Aligned = Builder.CreateICmpEQ(Align, llvm::ConstantInt::get(IntPtrTy, 0)); - Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment)); + if (Aligned != True) +Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment)); } } Index: test/CodeGenCXX/ubsan-suppress-checks.cpp === --- test/CodeGenCXX/ubsan-suppress-checks.cpp +++ test/CodeGenCXX/ubsan-suppress-checks.cpp @@ -17,6 +17,16 @@ // CHECK: ret void } +// CHECK-LABEL: define void @_Z31use_us16_aligned_array_elementsv +void use_us16_aligned_array_elements() { + static const unsigned short Arr[] = {0, 1, 2}; + auto use_array = [](const unsigned short(&X)[3]) -> void {}; + use_array(Arr); + + // CHECK-NOT: and i64 {{.*}}, !nosanitize + // CHECK: ret void +} + struct A { int foo; @@ -229,4 +239,5 @@ d->load_member_3(); load_non_null_pointers(); + use_us16_aligned_array_elements(); } Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -618,6 +618,7 @@ auto PtrToAlloca = dyn_cast(Ptr->stripPointerCastsNoFollowAliases()); + llvm::Value *True = llvm::ConstantInt::getTrue(getLLVMContext()); llvm::Value *IsNonNull = nullptr; bool IsGuaranteedNonNull = SkippedChecks.has(SanitizerKind::Null) || PtrToAlloca; @@ -629,8 +630,7 @@ // The IR builder can constant-fold the null check if the pointer points to // a constant. -IsGuaranteedNonNull = -IsNonNull == llvm::ConstantInt::getTrue(getLLVMContext()); +IsGuaranteedNonNull = IsNonNull == True; // Skip the null check if the pointer is known to be non-null. if (!IsGuaranteedNonNull) { @@ -684,7 +684,8 @@ PtrAsInt, llvm::ConstantInt::get(IntPtrTy, AlignVal - 1)); llvm::Value *Aligned = Builder.CreateICmpEQ(Align, llvm::ConstantInt::get(IntPtrTy, 0)); - Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment)); + if (Aligned != True) +Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment)); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values
rcraik created this revision. When the value specified for //n// in ordered(//n//) is larger than the number of loops a segmentation fault can occur in one of two ways when attempting to print out a diagnostic for an associated depend(sink : //vec//): 1. The iteration vector //vec// contains less than //n// items 2. The iteration vector //vec// contains a variable that is not a loop control variable This patch addresses both of these issues. https://reviews.llvm.org/D38049 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaOpenMP.cpp test/OpenMP/ordered_messages.cpp Index: test/OpenMP/ordered_messages.cpp === --- test/OpenMP/ordered_messages.cpp +++ test/OpenMP/ordered_messages.cpp @@ -270,5 +270,13 @@ } } +#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' clause}} + for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops after '#pragma omp for', but found only 1}} +#pragma omp ordered depend(sink : i) +int j; +#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop iteration variable}} +foo(); + } + return foo(); // expected-note {{in instantiation of function template specialization 'foo' requested here}} } Index: lib/Sema/SemaOpenMP.cpp === --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -10510,9 +10510,14 @@ if (!CurContext->isDependentContext() && DSAStack->getParentOrderedRegionParam() && DepCounter != DSAStack->isParentLoopControlVariable(D).first) { - Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) - << DSAStack->getParentLoopControlVariable( - DepCounter.getZExtValue()); + ValueDecl* VD = DSAStack->getParentLoopControlVariable( + DepCounter.getZExtValue()); + if (VD) { +Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) +<< 1 << VD; + } else { + Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 0; + } continue; } OpsOffs.push_back({RHS, OOK}); @@ -10545,8 +10550,9 @@ if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink && TotalDepCount > VarList.size() && -DSAStack->getParentOrderedRegionParam()) { - Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) +DSAStack->getParentOrderedRegionParam() && +DSAStack->getParentLoopControlVariable(VarList.size() + 1)) { + Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1 << DSAStack->getParentLoopControlVariable(VarList.size() + 1); } if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink && Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -8869,7 +8869,7 @@ def err_omp_depend_clause_thread_simd : Error< "'depend' clauses cannot be mixed with '%0' clause">; def err_omp_depend_sink_expected_loop_iteration : Error< - "expected %0 loop iteration variable">; + "expected%select{| %1}0 loop iteration variable">; def err_omp_depend_sink_unexpected_expr : Error< "unexpected expression: number of expressions is larger than the number of associated loops">; def err_omp_depend_sink_expected_plus_minus : Error< Index: test/OpenMP/ordered_messages.cpp === --- test/OpenMP/ordered_messages.cpp +++ test/OpenMP/ordered_messages.cpp @@ -270,5 +270,13 @@ } } +#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' clause}} + for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops after '#pragma omp for', but found only 1}} +#pragma omp ordered depend(sink : i) +int j; +#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop iteration variable}} +foo(); + } + return foo(); // expected-note {{in instantiation of function template specialization 'foo' requested here}} } Index: lib/Sema/SemaOpenMP.cpp === --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -10510,9 +10510,14 @@ if (!CurContext->isDependentContext() && DSAStack->getParentOrderedRegionParam() && DepCounter != DSAStack->isParentLoopControlVariable(D).first) { - Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) - << DSAStack->getParentLoopControlVariable( - DepCounter.getZExtValue()); + ValueDecl* VD = DSAStack->getParentLoopControlVariable( + DepCounter.getZExtValue()); + if (VD) { +Diag(ELoc, diag::err_
[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas
Rakete updated this revision to Diff 115881. Rakete added a project: clang. Rakete added a comment. Used better diagnostic id. https://reviews.llvm.org/D36357 Files: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseExprCXX.cpp test/Parser/cxx0x-lambda-expressions.cpp test/SemaCXX/new-delete-0x.cpp Index: test/SemaCXX/new-delete-0x.cpp === --- test/SemaCXX/new-delete-0x.cpp +++ test/SemaCXX/new-delete-0x.cpp @@ -34,6 +34,5 @@ void bad_deletes() { // 'delete []' is always array delete, per [expr.delete]p1. - // FIXME: Give a better diagnostic. - delete []{ return (int*)0; }(); // expected-error {{expected expression}} + delete []{ return (int*)0; }(); // expected-error{{missing parentheses around lambda expression}} } Index: test/Parser/cxx0x-lambda-expressions.cpp === --- test/Parser/cxx0x-lambda-expressions.cpp +++ test/Parser/cxx0x-lambda-expressions.cpp @@ -53,7 +53,7 @@ void delete_lambda(int *p) { delete [] p; delete [] (int*) { new int }; // ok, compound-literal, not lambda -delete [] { return new int; } (); // expected-error{{expected expression}} +delete [] { return new int; } (); // expected-error{{missing parentheses around lambda expression}} delete [&] { return new int; } (); // ok, lambda } Index: lib/Parse/ParseExprCXX.cpp === --- lib/Parse/ParseExprCXX.cpp +++ lib/Parse/ParseExprCXX.cpp @@ -2890,15 +2890,39 @@ // [Footnote: A lambda expression with a lambda-introducer that consists // of empty square brackets can follow the delete keyword if // the lambda expression is enclosed in parentheses.] -// FIXME: Produce a better diagnostic if the '[]' is unambiguously a -//lambda-introducer. -ArrayDelete = true; -BalancedDelimiterTracker T(*this, tok::l_square); +TentativeParsingAction TPA(*this); -T.consumeOpen(); -T.consumeClose(); -if (T.getCloseLocation().isInvalid()) - return ExprError(); +// Basic lookahead to check if we have a lambda expression. If we +// encounter two braces with a semicolon, we can be pretty sure +// that this is a lambda, not say a compound literal. +if (!SkipUntil(tok::l_brace, SkipUntilFlags::StopAtSemi) || +(NextToken().isNot(tok::r_brace) && !SkipUntil(tok::semi)) || +!SkipUntil(tok::r_brace, SkipUntilFlags::StopAtSemi)) { + TPA.Revert(); + ArrayDelete = true; + BalancedDelimiterTracker T(*this, tok::l_square); + + T.consumeOpen(); + T.consumeClose(); + if (T.getCloseLocation().isInvalid()) +return ExprError(); +} else { + TPA.Revert(); + + // Warn if the non-capturing lambda isn't surrounded by parenthesis + // to disambiguate it from 'delete[]'. + ExprResult Lambda = TryParseLambdaExpression(); + if (!Lambda.isInvalid()) { +SourceLocation StartLoc = Lambda.get()->getLocStart(); +Diag(StartLoc, diag::err_lambda_after_delete) + << SourceRange(StartLoc, Lambda.get()->getLocEnd()); + +// Evaluate any postfix expressions used on the lambda. +Lambda = ParsePostfixExpressionSuffix(Lambda); +return Actions.ActOnCXXDelete(Start, UseGlobal, /*ArrayForm=*/false, + Lambda.get()); + } +} } ExprResult Operand(ParseCastExpression(false)); Index: include/clang/Basic/DiagnosticParseKinds.td === --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -99,6 +99,8 @@ InGroup, DefaultIgnore; def ext_alignof_expr : ExtWarn< "%0 applied to an expression is a GNU extension">, InGroup; +def err_lambda_after_delete : Error< + "missing parentheses around lambda expression">; def warn_microsoft_dependent_exists : Warning< "dependent %select{__if_not_exists|__if_exists}0 declarations are ignored">, Index: test/SemaCXX/new-delete-0x.cpp === --- test/SemaCXX/new-delete-0x.cpp +++ test/SemaCXX/new-delete-0x.cpp @@ -34,6 +34,5 @@ void bad_deletes() { // 'delete []' is always array delete, per [expr.delete]p1. - // FIXME: Give a better diagnostic. - delete []{ return (int*)0; }(); // expected-error {{expected expression}} + delete []{ return (int*)0; }(); // expected-error{{missing parentheses around lambda expression}} } Index: test/Parser/cxx0x-lambda-expressions.cpp === --- test/Parser/cxx0x-lambda-expressions.cpp +++ test/Parser/cxx0x-lambda-expressions.cpp @@ -53,7 +53,7 @@ void delete_lambda(int *p) { delete [] p; delete [] (int*) { new int }; // ok, compound-literal, not
[PATCH] D37955: [libcxx] Fix invert negative bracket match.
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. All the tests pass now (on Mac OS) Thanks! https://reviews.llvm.org/D37955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8605 - return Match; + const char *Cmp; // Simplified, pretty-printed comparison string + // Similar to E->getOpcodeStr(), but with extra 0 on either LHS or RHS I'd drop this comment entirely. Comment at: lib/Sema/SemaChecking.cpp:8608 + + if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { +Result = false; Instead of doing all of this complex logic, why not structure the code like the original and use '?:' to determine which diagnostic id to pass in? ``` if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { S.Diag(E->getOperatorLoc(), HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison : diag::warn_lunsigned_always_true_comparison) << "< 0" << "false" << LHS->getSourceRange() << RHS->getSourceRange(); ``` Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D38049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r313666 - Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc
Author: akaylor Date: Tue Sep 19 13:26:40 2017 New Revision: 313666 URL: http://llvm.org/viewvc/llvm-project?rev=313666&view=rev Log: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc Differential Revision: https://reviews.llvm.org/D37042 Added: cfe/trunk/test/CodeGen/nullptr-arithmetic.c cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Modified: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/Expr.cpp cfe/trunk/lib/CodeGen/CGExprScalar.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/pointer-addition.c Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=313666&r1=313665&r2=313666&view=diff == --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Tue Sep 19 13:26:40 2017 @@ -3126,6 +3126,12 @@ public: return isShiftAssignOp(getOpcode()); } + // Return true if a binary operator using the specified opcode and operands + // would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized + // integer to a pointer. + static bool isNullPointerArithmeticExtension(ASTContext &Ctx, Opcode Opc, + Expr *LHS, Expr *RHS); + static bool classof(const Stmt *S) { return S->getStmtClass() >= firstBinaryOperatorConstant && S->getStmtClass() <= lastBinaryOperatorConstant; Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313666&r1=313665&r2=313666&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 13:26:40 2017 @@ -338,6 +338,7 @@ def NonPODVarargs : DiagGroup<"non-pod-v def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>; def : DiagGroup<"nonportable-cfstrings">; def NonVirtualDtor : DiagGroup<"non-virtual-dtor">; +def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">; def : DiagGroup<"effc++", [NonVirtualDtor]>; def OveralignedType : DiagGroup<"over-aligned">; def AlignedAllocationUnavailable : DiagGroup<"aligned-allocation-unavailable">; @@ -706,7 +707,8 @@ def Extra : DiagGroup<"extra", [ SemiBeforeMethodBody, MissingMethodReturnType, SignCompare, -UnusedParameter +UnusedParameter, +NullPointerArithmetic ]>; def Most : DiagGroup<"most", [ Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313666&r1=313665&r2=313666&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 13:26:40 2017 @@ -5501,6 +5501,12 @@ def err_offsetof_field_of_virtual_base : def warn_sub_ptr_zero_size_types : Warning< "subtraction of pointers to type %0 of zero size has undefined behavior">, InGroup; +def warn_pointer_arith_null_ptr : Warning< + "performing pointer arithmetic on a null pointer has undefined behavior%select{| if the offset is nonzero}0">, + InGroup, DefaultIgnore; +def warn_gnu_null_ptr_arith : Warning< + "arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension">, + InGroup, DefaultIgnore; def warn_floatingpoint_eq : Warning< "comparing floating point with == or != is unsafe">, Modified: cfe/trunk/lib/AST/Expr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=313666&r1=313665&r2=313666&view=diff == --- cfe/trunk/lib/AST/Expr.cpp (original) +++ cfe/trunk/lib/AST/Expr.cpp Tue Sep 19 13:26:40 2017 @@ -1829,6 +1829,45 @@ OverloadedOperatorKind BinaryOperator::g return OverOps[Opc]; } +bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx, + Opcode Opc, + Expr *LHS, Expr *RHS) { + if (Opc != BO_Add) +return false; + + // Check that we have one pointer and one integer operand. + Expr *PExp; + Expr *IExp; + if (LHS->getType()->isPointerType()) { +if (!RHS->getType()->isIntegerType()) + return false; +PExp = LHS; +IExp = RHS; + } else if (RHS->getType()->isPointerType()) { +if (!LHS->getType()->isIntegerType()) + return false; +PExp = RHS; +IExp = LHS; + } else { +return false; + } + + // Check that the pointer is a nullptr. + if (!PExp->IgnoreParenCasts() + ->i
[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc
This revision was automatically updated to reflect the committed changes. Closed by commit rL313666: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc (authored by akaylor). Changed prior to commit: https://reviews.llvm.org/D37042?vs=115262&id=115889#toc Repository: rL LLVM https://reviews.llvm.org/D37042 Files: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/Expr.cpp cfe/trunk/lib/CodeGen/CGExprScalar.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/CodeGen/nullptr-arithmetic.c cfe/trunk/test/Sema/pointer-addition.c cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp === --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp @@ -2678,6 +2678,30 @@ unsigned width = cast(index->getType())->getBitWidth(); auto &DL = CGF.CGM.getDataLayout(); auto PtrTy = cast(pointer->getType()); + + // Some versions of glibc and gcc use idioms (particularly in their malloc + // routines) that add a pointer-sized integer (known to be a pointer value) + // to a null pointer in order to cast the value back to an integer or as + // part of a pointer alignment algorithm. This is undefined behavior, but + // we'd like to be able to compile programs that use it. + // + // Normally, we'd generate a GEP with a null-pointer base here in response + // to that code, but it's also UB to dereference a pointer created that + // way. Instead (as an acknowledged hack to tolerate the idiom) we will + // generate a direct cast of the integer value to a pointer. + // + // The idiom (p = nullptr + N) is not met if any of the following are true: + // + // The operation is subtraction. + // The index is not pointer-sized. + // The pointer type is not byte-sized. + // + if (BinaryOperator::isNullPointerArithmeticExtension(CGF.getContext(), + op.Opcode, + expr->getLHS(), + expr->getRHS())) +return CGF.Builder.CreateIntToPtr(index, pointer->getType()); + if (width != DL.getTypeSizeInBits(PtrTy)) { // Zero-extend or sign-extend the pointer value according to // whether the index is signed or not. Index: cfe/trunk/lib/AST/Expr.cpp === --- cfe/trunk/lib/AST/Expr.cpp +++ cfe/trunk/lib/AST/Expr.cpp @@ -1829,6 +1829,45 @@ return OverOps[Opc]; } +bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx, + Opcode Opc, + Expr *LHS, Expr *RHS) { + if (Opc != BO_Add) +return false; + + // Check that we have one pointer and one integer operand. + Expr *PExp; + Expr *IExp; + if (LHS->getType()->isPointerType()) { +if (!RHS->getType()->isIntegerType()) + return false; +PExp = LHS; +IExp = RHS; + } else if (RHS->getType()->isPointerType()) { +if (!LHS->getType()->isIntegerType()) + return false; +PExp = RHS; +IExp = LHS; + } else { +return false; + } + + // Check that the pointer is a nullptr. + if (!PExp->IgnoreParenCasts() + ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull)) +return false; + + // Check that the pointee type is char-sized. + const PointerType *PTy = PExp->getType()->getAs(); + if (!PTy || !PTy->getPointeeType()->isCharType()) +return false; + + // Check that the integer type is pointer-sized. + if (Ctx.getTypeSize(IExp->getType()) != Ctx.getTypeSize(PExp->getType())) +return false; + + return true; +} InitListExpr::InitListExpr(const ASTContext &C, SourceLocation lbraceloc, ArrayRef initExprs, SourceLocation rbraceloc) : Expr(InitListExprClass, QualType(), VK_RValue, OK_Ordinary, false, false, Index: cfe/trunk/lib/Sema/SemaExpr.cpp === --- cfe/trunk/lib/Sema/SemaExpr.cpp +++ cfe/trunk/lib/Sema/SemaExpr.cpp @@ -8572,6 +8572,21 @@ << 0 /* one pointer */ << Pointer->getSourceRange(); } +/// \brief Diagnose invalid arithmetic on a null pointer. +/// +/// If \p IsGNUIdiom is true, the operation is using the 'p = (i8*)nullptr + n' +/// idiom, which we recognize as a GNU extension. +/// +static void diagnoseArithmeticOnNullPointer(Sema &S, SourceLocation Loc, +Expr *Pointer, bool IsGNUIdiom) { + if (IsGNUIdiom) +S.Diag(Loc, diag::warn_gnu_null_ptr_arith) + << Pointer->getSourceRange(); + else +S.Diag(Loc, diag::warn_pointer_arith_null_ptr) + << S.getLangOpts().CPlusPlus << Pointer->getSourceRange(); +} + /// \brief Di
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
lebedev.ri updated this revision to Diff 115890. lebedev.ri added a comment. Simplify `CheckTautologicalComparisonWithZero()` as per review notes. Less LOC. Repository: rL LLVM https://reviews.llvm.org/D37629 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/compare.c test/Sema/tautological-unsigned-enum-zero-compare.c Index: test/Sema/tautological-unsigned-enum-zero-compare.c === --- /dev/null +++ test/Sema/tautological-unsigned-enum-zero-compare.c @@ -0,0 +1,46 @@ +// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s + +int main() { + enum A { A_foo, A_bar }; + enum A a; + +#ifdef TEST + if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} +return 0; + if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} +return 0; + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} +return 0; + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} +return 0; + if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} +return 0; + if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} +return 0; + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} +return 0; + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} +return 0; +#else + // expected-no-diagnostics + if (a < 0) +return 0; + if (a >= 0) +return 0; + if (0 <= a) +return 0; + if (0 > a) +return 0; + if (a < 0U) +return 0; + if (a >= 0U) +return 0; + if (0U <= a) +return 0; + if (0U > a) +return 0; +#endif + + return 1; +} Index: test/Sema/compare.c === --- test/Sema/compare.c +++ test/Sema/compare.c @@ -308,8 +308,59 @@ int rdar8511238() { enum A { A_foo, A_bar }; enum A a; + + if (a == 0) + return 0; + if (a != 0) + return 0; if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} -return 0; + return 0; + if (a <= 0) + return 0; + if (a > 0) + return 0; + if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} + return 0; + + if (0 == a) + return 0; + if (0 != a) + return 0; + if (0 < a) + return 0; + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} + return 0; + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} + return 0; + if (0 >= a) + return 0; + + if (a == 0U) + return 0; + if (a != 0U) + return 0; + if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} + return 0; + if (a <= 0U) + return 0; + if (a > 0U) + return 0; + if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} + return 0; + + if (0U == a) + return 0; + if (0U != a) + return 0; + if (0U < a) + return 0; + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} + return 0; + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} + return 0; + if (0U >= a) + return 0; + return 20; } Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -8583,31 +8583,39 @@ // bool values are handled by DiagnoseOutOfRangeComparison(). - BinaryOperatorKind op = E->getOpcode(); + BinaryOperatorKind Op = E->getOpcode(); if (E->isValueDependent()) return false; Expr *LHS = E->getLHS(); Expr *RHS = E->getRHS(); bool Match = true; - if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { -S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison) - << "< 0" << "false" << HasEnumType(LHS) - << LHS->getSourceRange() << RHS->getSourceRange(); - } else if (op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { -S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison) - << ">= 0" << "true" << HasEnumType(LHS) - << LHS->getSourceRange() << RHS->getSourceRange(); - } else if (op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) { -S.Diag(E->getOperatorLoc(), diag::warn_runsigned_always_true_comparison) - << "0 >" << "false" << HasEnumType(RHS) - << LHS->getSourceRange() << RHS->get
[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions
weimingz updated this revision to Diff 115891. weimingz added a comment. Moved the inclusion from stdexcept.cpp into refstring.h For exception.cpp and new.cpp, I think it's better to keep the inclusion there. Those files include new_handler_fallback.ipp, exception_fallback.ipp, which need the header file. Including it in those .ipp requires relative path as "../../include/atomic_support.h" and I tried to avoid that. https://reviews.llvm.org/D35235 Files: src/exception.cpp src/include/atomic_support.h src/include/refstring.h src/locale.cpp src/new.cpp src/support/runtime/exception_fallback.ipp src/support/runtime/new_handler_fallback.ipp Index: src/support/runtime/new_handler_fallback.ipp === --- src/support/runtime/new_handler_fallback.ipp +++ src/support/runtime/new_handler_fallback.ipp @@ -15,13 +15,13 @@ new_handler set_new_handler(new_handler handler) _NOEXCEPT { -return __sync_lock_test_and_set(&__new_handler, handler); +return __libcpp_atomic_exchange(&__new_handler, handler); } new_handler get_new_handler() _NOEXCEPT { -return __sync_fetch_and_add(&__new_handler, nullptr); +return __libcpp_atomic_load(&__new_handler); } } // namespace std Index: src/support/runtime/exception_fallback.ipp === --- src/support/runtime/exception_fallback.ipp +++ src/support/runtime/exception_fallback.ipp @@ -20,13 +20,13 @@ unexpected_handler set_unexpected(unexpected_handler func) _NOEXCEPT { - return __sync_lock_test_and_set(&__unexpected_handler, func); + return __libcpp_atomic_exchange(&__unexpected_handler, func); } unexpected_handler get_unexpected() _NOEXCEPT { - return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0); + return __libcpp_atomic_load(&__unexpected_handler); } @@ -41,14 +41,13 @@ terminate_handler set_terminate(terminate_handler func) _NOEXCEPT { - return __sync_lock_test_and_set(&__terminate_handler, func); + return __libcpp_atomic_exchange(&__terminate_handler, func); } terminate_handler get_terminate() _NOEXCEPT { - return __sync_fetch_and_add(&__terminate_handler, (terminate_handler)0); - + return __libcpp_atomic_load(&__terminate_handler); } #ifndef __EMSCRIPTEN__ // We provide this in JS Index: src/new.cpp === --- src/new.cpp +++ src/new.cpp @@ -12,6 +12,7 @@ #include #include "new" +#include "include/atomic_support.h" #if defined(_LIBCPP_ABI_MICROSOFT) // nothing todo Index: src/locale.cpp === --- src/locale.cpp +++ src/locale.cpp @@ -36,6 +36,7 @@ #endif #include #include +#include "include/atomic_support.h" #include "__undef_macros" // On Linux, wint_t and wchar_t have different signed-ness, and this causes @@ -667,7 +668,7 @@ void locale::id::__init() { -__id_ = __sync_add_and_fetch(&__next_id, 1); +__id_ = __libcpp_atomic_add(&__next_id, 1); } // template <> class collate_byname Index: src/include/refstring.h === --- src/include/refstring.h +++ src/include/refstring.h @@ -18,6 +18,7 @@ #include #include #endif +#include "atomic_support.h" _LIBCPP_BEGIN_NAMESPACE_STD @@ -83,19 +84,19 @@ : __imp_(s.__imp_) { if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); } inline __libcpp_refstring& __libcpp_refstring::operator=(__libcpp_refstring const& s) _NOEXCEPT { bool adjust_old_count = __uses_refcount(); struct _Rep_base *old_rep = rep_from_data(__imp_); __imp_ = s.__imp_; if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); if (adjust_old_count) { -if (__sync_add_and_fetch(&old_rep->count, count_t(-1)) < 0) +if (__libcpp_atomic_add(&old_rep->count, count_t(-1)) < 0) { ::operator delete(old_rep); } @@ -107,7 +108,7 @@ __libcpp_refstring::~__libcpp_refstring() { if (__uses_refcount()) { _Rep_base* rep = rep_from_data(__imp_); -if (__sync_add_and_fetch(&rep->count, count_t(-1)) < 0) { +if (__libcpp_atomic_add(&rep->count, count_t(-1)) < 0) { ::operator delete(rep); } } Index: src/include/atomic_support.h === --- src/include/atomic_support.h +++ src/include/atomic_support.h @@ -16,6 +16,7 @@ #if defined(__clang__) && __has_builtin(__atomic_load_n) \ && __has_builtin(__atomic_store_n)\ && __has_builtin(__atomic_add_fetch) \ + && __
[PATCH] D37881: [Sema] Prevent InstantiateClass from checking unrelated exception specs.
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D37881#871556, @vsapsai wrote: > I don't like name `SavePendingDelayedStateRAII` as too vague. Hope you'll be > able to suggest something better or I'll have better ideas later. I like everything about this patch other than the name of the class :) I think we need to clearly identify what state we're saving here and why. It's the state that is pending from a class that we're currently in the middle of parsing, so how about something like `SavePendingParsedClassStateRAII`? https://reviews.llvm.org/D37881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r313675 - [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values
Author: rcraik Date: Tue Sep 19 14:04:23 2017 New Revision: 313675 URL: http://llvm.org/viewvc/llvm-project?rev=313675&view=rev Log: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values When the value specified for n in ordered(n) is larger than the number of loops a segmentation fault can occur in one of two ways when attempting to print out a diagnostic for an associated depend(sink : vec): 1) The iteration vector vec contains less than n items 2) The iteration vector vec contains a variable that is not a loop control variable This patch addresses both of these issues. Differential Revision: https://reviews.llvm.org/D38049 Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaOpenMP.cpp cfe/trunk/test/OpenMP/ordered_messages.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313675&r1=313674&r2=313675&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 14:04:23 2017 @@ -8875,7 +8875,7 @@ def note_omp_critical_no_hint : Note< def err_omp_depend_clause_thread_simd : Error< "'depend' clauses cannot be mixed with '%0' clause">; def err_omp_depend_sink_expected_loop_iteration : Error< - "expected %0 loop iteration variable">; + "expected%select{| %1}0 loop iteration variable">; def err_omp_depend_sink_unexpected_expr : Error< "unexpected expression: number of expressions is larger than the number of associated loops">; def err_omp_depend_sink_expected_plus_minus : Error< Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=313675&r1=313674&r2=313675&view=diff == --- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original) +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Sep 19 14:04:23 2017 @@ -10510,9 +10510,14 @@ Sema::ActOnOpenMPDependClause(OpenMPDepe if (!CurContext->isDependentContext() && DSAStack->getParentOrderedRegionParam() && DepCounter != DSAStack->isParentLoopControlVariable(D).first) { - Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) - << DSAStack->getParentLoopControlVariable( - DepCounter.getZExtValue()); + ValueDecl* VD = DSAStack->getParentLoopControlVariable( + DepCounter.getZExtValue()); + if (VD) { +Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) +<< 1 << VD; + } else { + Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 0; + } continue; } OpsOffs.push_back({RHS, OOK}); @@ -10545,8 +10550,9 @@ Sema::ActOnOpenMPDependClause(OpenMPDepe if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink && TotalDepCount > VarList.size() && -DSAStack->getParentOrderedRegionParam()) { - Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) +DSAStack->getParentOrderedRegionParam() && +DSAStack->getParentLoopControlVariable(VarList.size() + 1)) { + Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1 << DSAStack->getParentLoopControlVariable(VarList.size() + 1); } if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink && Modified: cfe/trunk/test/OpenMP/ordered_messages.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/ordered_messages.cpp?rev=313675&r1=313674&r2=313675&view=diff == --- cfe/trunk/test/OpenMP/ordered_messages.cpp (original) +++ cfe/trunk/test/OpenMP/ordered_messages.cpp Tue Sep 19 14:04:23 2017 @@ -270,5 +270,13 @@ int k; } } +#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' clause}} + for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops after '#pragma omp for', but found only 1}} +#pragma omp ordered depend(sink : i) +int j; +#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop iteration variable}} +foo(); + } + return foo(); // expected-note {{in instantiation of function template specialization 'foo' requested here}} } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values
This revision was automatically updated to reflect the committed changes. Closed by commit rL313675: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values (authored by rcraik). Changed prior to commit: https://reviews.llvm.org/D38049?vs=115876&id=115901#toc Repository: rL LLVM https://reviews.llvm.org/D38049 Files: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaOpenMP.cpp cfe/trunk/test/OpenMP/ordered_messages.cpp Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp === --- cfe/trunk/lib/Sema/SemaOpenMP.cpp +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp @@ -10510,9 +10510,14 @@ if (!CurContext->isDependentContext() && DSAStack->getParentOrderedRegionParam() && DepCounter != DSAStack->isParentLoopControlVariable(D).first) { - Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) - << DSAStack->getParentLoopControlVariable( - DepCounter.getZExtValue()); + ValueDecl* VD = DSAStack->getParentLoopControlVariable( + DepCounter.getZExtValue()); + if (VD) { +Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) +<< 1 << VD; + } else { + Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 0; + } continue; } OpsOffs.push_back({RHS, OOK}); @@ -10545,8 +10550,9 @@ if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink && TotalDepCount > VarList.size() && -DSAStack->getParentOrderedRegionParam()) { - Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) +DSAStack->getParentOrderedRegionParam() && +DSAStack->getParentLoopControlVariable(VarList.size() + 1)) { + Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1 << DSAStack->getParentLoopControlVariable(VarList.size() + 1); } if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink && Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -8875,7 +8875,7 @@ def err_omp_depend_clause_thread_simd : Error< "'depend' clauses cannot be mixed with '%0' clause">; def err_omp_depend_sink_expected_loop_iteration : Error< - "expected %0 loop iteration variable">; + "expected%select{| %1}0 loop iteration variable">; def err_omp_depend_sink_unexpected_expr : Error< "unexpected expression: number of expressions is larger than the number of associated loops">; def err_omp_depend_sink_expected_plus_minus : Error< Index: cfe/trunk/test/OpenMP/ordered_messages.cpp === --- cfe/trunk/test/OpenMP/ordered_messages.cpp +++ cfe/trunk/test/OpenMP/ordered_messages.cpp @@ -270,5 +270,13 @@ } } +#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' clause}} + for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops after '#pragma omp for', but found only 1}} +#pragma omp ordered depend(sink : i) +int j; +#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop iteration variable}} +foo(); + } + return foo(); // expected-note {{in instantiation of function template specialization 'foo' requested here}} } Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp === --- cfe/trunk/lib/Sema/SemaOpenMP.cpp +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp @@ -10510,9 +10510,14 @@ if (!CurContext->isDependentContext() && DSAStack->getParentOrderedRegionParam() && DepCounter != DSAStack->isParentLoopControlVariable(D).first) { - Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) - << DSAStack->getParentLoopControlVariable( - DepCounter.getZExtValue()); + ValueDecl* VD = DSAStack->getParentLoopControlVariable( + DepCounter.getZExtValue()); + if (VD) { +Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) +<< 1 << VD; + } else { + Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 0; + } continue; } OpsOffs.push_back({RHS, OOK}); @@ -10545,8 +10550,9 @@ if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink && TotalDepCount > VarList.size() && -DSAStack->getParentOrderedRegionParam()) { - Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) +DSAStack->getParentOrderedRegionParam() && +DSAStack->getParentLoopControlVariable(VarList.size() + 1)) { + Diag(EndLoc, diag::err_omp_depend_sink
r313677 - [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
Author: lebedevri Date: Tue Sep 19 14:11:35 2017 New Revision: 313677 URL: http://llvm.org/viewvc/llvm-project?rev=313677&view=rev Log: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare Summary: As requested by Sam McCall: > Enums (not new I guess). Typical case: if (enum < 0 || enum > MAX) > The warning strongly suggests that the enum < 0 check has no effect > (for enums with nonnegative ranges). > Clang doesn't seem to optimize such checks out though, and they seem > likely to catch bugs in some cases. Yes, only if there's UB elsewhere, > but I assume not optimizing out these checks indicates a deliberate > decision to stay somewhat compatible with a technically-incorrect > mental model. > If this is the case, should we move these to a > -Wtautological-compare-enum subcategory? Reviewers: rjmccall, rsmith, aaron.ballman, sammccall, bkramer, djasper Reviewed By: aaron.ballman Subscribers: jroelofs, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D37629 Added: cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/compare.c Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313677&r1=313676&r2=313677&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 14:11:35 2017 @@ -429,12 +429,14 @@ def StringPlusInt : DiagGroup<"string-pl def StringPlusChar : DiagGroup<"string-plus-char">; def StrncatSize : DiagGroup<"strncat-size">; def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">; +def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">; def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalUnsignedZeroCompare, + TautologicalUnsignedEnumZeroCompare, TautologicalOutOfRangeCompare, TautologicalPointerCompare, TautologicalOverlapCompare, Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313677&r1=313676&r2=313677&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 14:11:35 2017 @@ -5906,19 +5906,26 @@ def note_typecheck_assign_const : Note< "member function %q1 is declared const here|" "%select{|nested }1data member %2 declared const here}0">; +def warn_lunsigned_always_true_comparison : Warning< + "comparison of unsigned expression %0 is always %select{false|true}1">, + InGroup; +def warn_runsigned_always_true_comparison : Warning< + "comparison of %0 unsigned expression is always %select{false|true}1">, + InGroup; +def warn_lunsigned_enum_always_true_comparison : Warning< + "comparison of unsigned enum expression %0 is always %select{false|true}1">, + InGroup; +def warn_runsigned_enum_always_true_comparison : Warning< + "comparison of %0 unsigned enum expression is always %select{false|true}1">, + InGroup; + def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, InGroup, DefaultIgnore; -def warn_lunsigned_always_true_comparison : Warning< - "comparison of unsigned%select{| enum}2 expression %0 is always %1">, - InGroup; def warn_out_of_range_compare : Warning< "comparison of %select{constant %0|true|false}1 with " "%select{expression of type %2|boolean expression}3 is always " "%select{false|true}4">, InGroup; -def warn_runsigned_always_true_comparison : Warning< - "comparison of %0 unsigned%select{| enum}2 expression is always %1">, - InGroup; def warn_comparison_of_mixed_enum_types : Warning< "comparison of two values with different enumeration types" "%diff{ ($ and $)|}0,1">, Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=313677&r1=313676&r2=313677&view=diff == --- cfe/tru
Re: r312750 - [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.
On Fri, Sep 8, 2017 at 7:10 PM, Roman Lebedev wrote: > On Fri, Sep 8, 2017 at 3:26 PM, Roman Lebedev wrote: >> On Fri, Sep 8, 2017 at 2:48 PM, Sam McCall wrote: >> Hi. >> >>> Nice fix! >> Thank you! >> >>> It catches a lot of new cases on our codebase, all technically >>> correct so far. >>> >>> A couple of issues though: >>> A) Rollout - until we've completely cleaned up, we need to disable >>> -Wtautological-compare entirely, which is a valuable check. I imagine anyone >>> else using -Werror is in the same boat. >>> What do you think about putting the new warnings behind a subcategory? (e.g. >>> -Wtautological-compare-unsigned-zero, implied by -Wtautological-compare) >>> It's an ugly artifact of the history here, but allows this fix to be rolled >>> out in a controlled way. >> https://reviews.llvm.org/D37620 > And landed. > >>> B) Enums (not new I guess). Typical case: if (enum < 0 || enum > MAX) >>> The warning strongly suggests that the enum < 0 check has no effect (for >>> enums with nonnegative ranges). >>> Clang doesn't seem to optimize such checks out though, and they seem likely >>> to catch bugs in some cases. Yes, only if there's UB elsewhere, but I assume >>> not optimizing out these checks indicates a deliberate decision to stay >>> somewhat compatible with a technically-incorrect mental model. >>> If this is the case, should we move these to a -Wtautological-compare-enum >>> subcategory? >> (Did not look at this yet) > https://reviews.llvm.org/D37629 i hope that is what you meant. And landed, too. Next will try to work on the second part of the fix for https://bugs.llvm.org/show_bug.cgi?id=34147 >> Roman. > Roman. Roman. >>> On Fri, Sep 8, 2017 at 12:14 AM, Roman Lebedev via cfe-commits >>> wrote: Author: lebedevri Date: Thu Sep 7 15:14:25 2017 New Revision: 312750 URL: http://llvm.org/viewvc/llvm-project?rev=312750&view=rev Log: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S. Summary: This is a first half(?) of a fix for the following bug: https://bugs.llvm.org/show_bug.cgi?id=34147 (gcc -Wtype-limits) GCC's -Wtype-limits does warn on comparison of unsigned value with signed zero (as in, with 0), but clang only warns if the zero is unsigned (i.e. 0U). Also, be careful not to double-warn, or falsely warn on comparison of signed/fp variable and signed 0. Yes, all these testcases are needed. Testing: $ ninja check-clang-sema check-clang-semacxx Also, no new warnings for clang stage-2 build. Reviewers: rjmccall, rsmith, aaron.ballman Reviewed By: rjmccall Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D37565 Modified: cfe/trunk/docs/ReleaseNotes.rst cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/compare.c cfe/trunk/test/Sema/outof-range-constant-compare.c cfe/trunk/test/SemaCXX/compare.cpp Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=312750&r1=312749&r2=312750&view=diff == --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Thu Sep 7 15:14:25 2017 @@ -71,6 +71,13 @@ Improvements to Clang's diagnostics errors/warnings, as the system frameworks might add a method with the same selector which could make the message send to ``id`` ambiguous. +- ``-Wtautological-compare`` now warns when comparing an unsigned integer and 0 + regardless of whether the constant is signed or unsigned." + +- ``-Wtautological-compare`` now warns about comparing a signed integer and 0 + when the signed integer is coerced to an unsigned type for the comparison. + ``-Wsign-compare`` was adjusted not to warn in this case. + Non-comprehensive list of changes in this release - Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=312750&r1=312749&r2=312750&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Sep 7 15:14:25 2017 @@ -8567,32 +8567,51 @@ bool HasEnumType(Expr *E) { return E->getType()->isEnumeralType(); } -void CheckTrivialUnsignedComparison(Sema &S, BinaryOperator *E) { +bool isNonBooleanUnsignedValue(Expr *E) { + // We are checking that the expression is not known to have boolean value, + // is an integer type; and is either unsigned after implicit casts, +
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
This revision was automatically updated to reflect the committed changes. Closed by commit rL313677: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare (authored by lebedevri). Changed prior to commit: https://reviews.llvm.org/D37629?vs=115890&id=115903#toc Repository: rL LLVM https://reviews.llvm.org/D37629 Files: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/compare.c cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c Index: cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c === --- cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c +++ cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c @@ -0,0 +1,46 @@ +// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s + +int main() { + enum A { A_foo, A_bar }; + enum A a; + +#ifdef TEST + if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} +return 0; + if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} +return 0; + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} +return 0; + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} +return 0; + if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} +return 0; + if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} +return 0; + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} +return 0; + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} +return 0; +#else + // expected-no-diagnostics + if (a < 0) +return 0; + if (a >= 0) +return 0; + if (0 <= a) +return 0; + if (0 > a) +return 0; + if (a < 0U) +return 0; + if (a >= 0U) +return 0; + if (0U <= a) +return 0; + if (0U > a) +return 0; +#endif + + return 1; +} Index: cfe/trunk/test/Sema/compare.c === --- cfe/trunk/test/Sema/compare.c +++ cfe/trunk/test/Sema/compare.c @@ -308,8 +308,59 @@ int rdar8511238() { enum A { A_foo, A_bar }; enum A a; + + if (a == 0) + return 0; + if (a != 0) + return 0; if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} -return 0; + return 0; + if (a <= 0) + return 0; + if (a > 0) + return 0; + if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} + return 0; + + if (0 == a) + return 0; + if (0 != a) + return 0; + if (0 < a) + return 0; + if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} + return 0; + if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} + return 0; + if (0 >= a) + return 0; + + if (a == 0U) + return 0; + if (a != 0U) + return 0; + if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}} + return 0; + if (a <= 0U) + return 0; + if (a > 0U) + return 0; + if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}} + return 0; + + if (0U == a) + return 0; + if (0U != a) + return 0; + if (0U < a) + return 0; + if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}} + return 0; + if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}} + return 0; + if (0U >= a) + return 0; + return 20; } Index: cfe/trunk/lib/Sema/SemaChecking.cpp === --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -8583,31 +8583,39 @@ // bool values are handled by DiagnoseOutOfRangeComparison(). - BinaryOperatorKind op = E->getOpcode(); + BinaryOperatorKind Op = E->getOpcode(); if (E->isValueDependent()) return false; Expr *LHS = E->getLHS(); Expr *RHS = E->getRHS(); bool Match = true; - if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { -S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison) - << "< 0" << "false" << HasEnumType(LHS) - << LHS->getSourceRange() << RHS->getSourceRange(); - } else if (op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { -S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison) - << ">= 0" << "true" << HasEnumType(LHS) -
[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc
andrew.w.kaylor added a comment. This is breaking buildbots for 32-bit targets because the offset in 'nullptr + int8_t_N' is being implicitly cast to an int. That makes the sizeof(offset) == sizeof(ptr) check turn out differently than my tests were assuming. I can get the buildbots green quickly by taking out parts of the tests, but I just talked to Erich Keane about this and we think the right way to fix this long term is to stop checking for sizeof(offset) == sizeof(ptr) in the code that identifies the idiom, since that check is of dubious value and would be difficult to always get to behave the way I intended. Repository: rL LLVM https://reviews.llvm.org/D37042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36150: [clangd] LSP extension to switch between source/header file
Nebiroth added a comment. In https://reviews.llvm.org/D36150#867971, @ilya-biryukov wrote: > Overall looks good, but still needs at least a few tests. I have a test for this commit that uses included source and header files, would that be okay to do or should I generate files dynamically? If so, how can I accomplish this? https://reviews.llvm.org/D36150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r313683 - Revert "[Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare"
Author: lebedevri Date: Tue Sep 19 14:40:41 2017 New Revision: 313683 URL: http://llvm.org/viewvc/llvm-project?rev=313683&view=rev Log: Revert "[Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare" This reverts commit r313677. Buildbots fail with assertion failure Failing Tests (7): Clang :: Analysis/null-deref-ps.c Clang :: CodeGen/enum.c Clang :: Sema/compare.c Clang :: Sema/outof-range-constant-compare.c Clang :: Sema/tautological-unsigned-enum-zero-compare.c Clang :: Sema/tautological-unsigned-zero-compare.c Clang :: SemaCXX/compare.cpp Removed: cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/compare.c Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313683&r1=313682&r2=313683&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 14:40:41 2017 @@ -429,14 +429,12 @@ def StringPlusInt : DiagGroup<"string-pl def StringPlusChar : DiagGroup<"string-plus-char">; def StrncatSize : DiagGroup<"strncat-size">; def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">; -def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">; def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalUnsignedZeroCompare, - TautologicalUnsignedEnumZeroCompare, TautologicalOutOfRangeCompare, TautologicalPointerCompare, TautologicalOverlapCompare, Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313683&r1=313682&r2=313683&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 14:40:41 2017 @@ -5906,26 +5906,19 @@ def note_typecheck_assign_const : Note< "member function %q1 is declared const here|" "%select{|nested }1data member %2 declared const here}0">; -def warn_lunsigned_always_true_comparison : Warning< - "comparison of unsigned expression %0 is always %select{false|true}1">, - InGroup; -def warn_runsigned_always_true_comparison : Warning< - "comparison of %0 unsigned expression is always %select{false|true}1">, - InGroup; -def warn_lunsigned_enum_always_true_comparison : Warning< - "comparison of unsigned enum expression %0 is always %select{false|true}1">, - InGroup; -def warn_runsigned_enum_always_true_comparison : Warning< - "comparison of %0 unsigned enum expression is always %select{false|true}1">, - InGroup; - def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, InGroup, DefaultIgnore; +def warn_lunsigned_always_true_comparison : Warning< + "comparison of unsigned%select{| enum}2 expression %0 is always %1">, + InGroup; def warn_out_of_range_compare : Warning< "comparison of %select{constant %0|true|false}1 with " "%select{expression of type %2|boolean expression}3 is always " "%select{false|true}4">, InGroup; +def warn_runsigned_always_true_comparison : Warning< + "comparison of %0 unsigned%select{| enum}2 expression is always %1">, + InGroup; def warn_comparison_of_mixed_enum_types : Warning< "comparison of two values with different enumeration types" "%diff{ ($ and $)|}0,1">, Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=313683&r1=313682&r2=313683&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Sep 19 14:40:41 2017 @@ -8583,7 +8583,7 @@ bool CheckTautologicalComparisonWithZero // bool values are handled by DiagnoseOutOfRangeComparison(). - BinaryOperatorKind Op = E->getOpcode(); + BinaryOperatorKind op = E->getOpcode(); if (E->isValueDependent()) return false; @@ -8592,30 +8592,22 @@ bool CheckTautologicalCom
r313684 - Fix 32-bit buildbots by removing tests that are dependent on pointer-size comparisons.
Author: akaylor Date: Tue Sep 19 14:43:01 2017 New Revision: 313684 URL: http://llvm.org/viewvc/llvm-project?rev=313684&view=rev Log: Fix 32-bit buildbots by removing tests that are dependent on pointer-size comparisons. The recently behavior in the code that these tests were meant to be checking will be ammended as soon as a suitable change can be properly reviewed. Modified: cfe/trunk/test/CodeGen/nullptr-arithmetic.c cfe/trunk/test/Sema/pointer-addition.c cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Modified: cfe/trunk/test/CodeGen/nullptr-arithmetic.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/nullptr-arithmetic.c?rev=313684&r1=313683&r2=313684&view=diff == --- cfe/trunk/test/CodeGen/nullptr-arithmetic.c (original) +++ cfe/trunk/test/CodeGen/nullptr-arithmetic.c Tue Sep 19 14:43:01 2017 @@ -17,26 +17,18 @@ int8_t *test1(intptr_t n) { // CHECK: inttoptr // CHECK-NOT: getelementptr -// This doesn't meet the idiom because the offset type isn't pointer-sized. -int8_t *test2(int8_t n) { - return NULLPTRI8 + n; -} -// CHECK-LABEL: test2 -// CHECK: getelementptr -// CHECK-NOT: inttoptr - // This doesn't meet the idiom because the element type is larger than a byte. -int16_t *test3(intptr_t n) { +int16_t *test2(intptr_t n) { return (int16_t*)0 + n; } -// CHECK-LABEL: test3 +// CHECK-LABEL: test2 // CHECK: getelementptr // CHECK-NOT: inttoptr // This doesn't meet the idiom because the offset is subtracted. -int8_t* test4(intptr_t n) { +int8_t* test3(intptr_t n) { return NULLPTRI8 - n; } -// CHECK-LABEL: test4 +// CHECK-LABEL: test3 // CHECK: getelementptr // CHECK-NOT: inttoptr Modified: cfe/trunk/test/Sema/pointer-addition.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pointer-addition.c?rev=313684&r1=313683&r2=313684&view=diff == --- cfe/trunk/test/Sema/pointer-addition.c (original) +++ cfe/trunk/test/Sema/pointer-addition.c Tue Sep 19 14:43:01 2017 @@ -27,6 +27,4 @@ void a(S* b, void* c) { // Cases that don't match the GNU inttoptr idiom get a different warning. f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} - unsigned char j = (unsigned char)b; - f = (char*)0 + j; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} } Modified: cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp?rev=313684&r1=313683&r2=313684&view=diff == --- cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp (original) +++ cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Tue Sep 19 14:43:01 2017 @@ -2,7 +2,7 @@ #include -void f(intptr_t offset, int8_t b) { +void f(intptr_t offset) { // A zero offset from a nullptr is OK. char *f = (char*)nullptr + 0; int *g = (int*)0 + 0; @@ -13,7 +13,6 @@ void f(intptr_t offset, int8_t b) { // Cases that don't match the GNU inttoptr idiom get a different warning. f = (char*)0 - offset; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero}} g = (int*)0 + offset; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero}} - f = (char*)0 + b; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero}} } // Value-dependent pointer arithmetic should not produce a nullptr warning. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare
lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted due to buildbot failures. Need to investigate. Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions
EricWF accepted this revision. EricWF added inline comments. Comment at: src/include/atomic_support.h:153 +_ValueType old; +old = *__target; +*__target = __value; Initialize `old` on the same line it's declared on. https://reviews.llvm.org/D35235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc
efriedma added a comment. I agree, it doesn't add much value. Either way, though, please make sure you address the buildbot failures quickly. Repository: rL LLVM https://reviews.llvm.org/D37042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31140: [LLVMbugs] [Bug 18710] Only generate .ARM.exidx and .ARM.extab when needed in EHABI
efriedma added a comment. I would rather not make ARM baremetal do something different from every other target... On Linux, for most targets, we don't add the uwtable attribute by default; without the uwtable attribute, non-ARM backends (e.g. aarch64) only emit tables for functions which might unwind. Following this precedent is probably the most straightforward. MachO::IsUnwindTablesDefault does something similar to what you're doing here (except it doesn't depend on CCCIsCXX). But Apple platforms have to deal with ObjC exception handling, so I'm not sure that's the best precedent to follow. https://reviews.llvm.org/D31140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37954: Expand absolute system header paths when using -MD depfiles
thakis added a comment. I think for depfile generation, we generally try to be gcc-compatible (I can link to prior changes in this spirit), so I think this seems like a good thing to do to me. gcc only does this for system headers, yes? Does gcc support the non-negated spelling of this flag (-f-canonical-system-headers) too? If so, we should probably support that too for completeness, even if specifying it explicitly won't change behavior. https://reviews.llvm.org/D37954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r313693 - Add override for ClangDiagnosticHandler::isAnyRemarkEnabled()
Author: anemet Date: Tue Sep 19 16:00:59 2017 New Revision: 313693 URL: http://llvm.org/viewvc/llvm-project?rev=313693&view=rev Log: Add override for ClangDiagnosticHandler::isAnyRemarkEnabled() This is used by the new closure-based variant of OptimizationRemarkEmitter::emit(). Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=313693&r1=313692&r2=313693&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Tue Sep 19 16:00:59 2017 @@ -67,6 +67,12 @@ namespace clang { CodeGenOpts.OptimizationRemarkPattern->match(PassName)); } +bool isAnyRemarkEnabled() const override { + return (CodeGenOpts.OptimizationRemarkAnalysisPattern || + CodeGenOpts.OptimizationRemarkMissedPattern || + CodeGenOpts.OptimizationRemarkPattern); +} + private: const CodeGenOptions &CodeGenOpts; BackendConsumer *BackendCon; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32520: Support __fp16 vectors
bruno added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1042 +} + +assert(SrcElementTy->isFloatingPointTy() && What happens if the SrcElementTy is float and DstElementTy isn't? Seems like it will hit the assertion below. Comment at: lib/Sema/SemaExpr.cpp:7511 + // If this is a compound assignment, allow converting the RHS to the type + // of the LHS. + if (IsCompAssign && isVector(LHSType, Context.HalfTy)) { Since it seems that you're always doing the same conversion (the only variable input here is the number of elements), can you update the comment to mention the exact conversion? Comment at: lib/Sema/SemaExpr.cpp:8072 +/// Convert E, which is a vector, to a vector that has a different element +/// type. +static ExprResult convertVector(Expr *E, QualType ElementType, Sema &S) { What about `Convert vector E to a vector with the same number of elements but different element type`? Comment at: lib/Sema/SemaExpr.cpp:11316 +// This helper function promotes a binary operator's operands (which are of a +// half vector type) to a vector of floats and then truncates the result to +// a vector of either half or short. `which are of a half vector type` -> should be there an assertion below to make sure? Comment at: lib/Sema/SemaExpr.cpp:11329 + QualType BinOpResTy = RHS.get()->getType(); + if (isVector(ResultTy, Context.ShortTy)) +BinOpResTy = S.GetSignedVectorType(BinOpResTy); Can you add a comment explaining that a) this conversion from float -> int and b) it's needed in case the original binop is a comparison? Comment at: lib/Sema/SemaExpr.cpp:11537 + // Some of the binary operations require promoting operands of half vector + // and truncating the result. For now, we do this only when HalfArgsAndReturn + // is set (that is, when the target is arm or arm64). What about `of half vector and truncating the result` to `of half vector to float and truncating the result back to half` Comment at: lib/Sema/SemaExpr.cpp:11568 - if (CompResultTy.isNull()) + if (CompResultTy.isNull()) { +if (ConvertHalfVec) Please add a comment here explaining that this path only happens when it's a compound assignment. Comment at: lib/Sema/SemaExpr.cpp:11978 +// truncating the result. For now, we do this only when HalfArgsAndReturns +// is set (that is, when the target is arm or arm64). +ConvertHalfVec = This same logic is used elsewhere in the patch, perhaps factor it out into a static function? https://reviews.llvm.org/D32520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.
chandlerc added a comment. In https://reviews.llvm.org/D38042#875441, @aprantl wrote: > In https://reviews.llvm.org/D38042#875418, @chandlerc wrote: > > > Absolutely. I think the verifier should never, under any circumstances, > > mutate the IR. Think about it, with the current design if a pass corrupts > > the debug info the verifier may "hide" this by stripping it out rather than > > allowing us to find it. > > > Okay, I think I agree. Before I venture off implementing this, do you think > that separating out a StripBrokenDebugInfoPass that depends on the Verifier > is right way forward? I don't know what you mean by "depends on" and I'm not sure I'm the right person to say what the exact best design is... But I'd throw something together and start that review on llvm-commits where we can get folks more familiar w/ these details involved to figure it out. It at least seems to be in the right direction. My only concern is that passes are supposed to be able to rely on the verifier passing, and this wouldn't... Not sure how to handle that case. > > >> But auto-upgrade happens on *read*. If you want to add the debug info >> stripping to auto-upgrade, that's a reasonable discussion to have, and no >> change here will be required. There might be concerns about that on the LLVM >> side, I don't know. But the verifier (as well as running it here) does not >> seem like the right solution. > > Would splitting the VerifierPass into a VerifierPass and a > StripBrokenDebugInfoPass *together* with adding a -enable-llvm-verifier (an > explicit opposite of -disable-llvm-verifier) work for you? If you want a way to use the `clang` binary to strip broken debug info from a bitcode input, I would add a flag that does exactly this, and leave the verifier as an independent component. I don't know whether such a flag makes sense or not (Richard or other more deep in Clang-land would have a better feel than I would). But I think that whether the verifier is enabled or not should be orthogonal from any debug info stripping. Stripping the debug info might impact whether something verifies, but the flags should be completely independent. However, if the debug info stripping ends up as part of auto upgrade, all of this becomes much simpler to think about. https://reviews.llvm.org/D38042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions
weimingz updated this revision to Diff 115918. weimingz added a comment. minor change https://reviews.llvm.org/D35235 Files: src/exception.cpp src/include/atomic_support.h src/include/refstring.h src/locale.cpp src/new.cpp src/support/runtime/exception_fallback.ipp src/support/runtime/new_handler_fallback.ipp Index: src/support/runtime/new_handler_fallback.ipp === --- src/support/runtime/new_handler_fallback.ipp +++ src/support/runtime/new_handler_fallback.ipp @@ -15,13 +15,13 @@ new_handler set_new_handler(new_handler handler) _NOEXCEPT { -return __sync_lock_test_and_set(&__new_handler, handler); +return __libcpp_atomic_exchange(&__new_handler, handler); } new_handler get_new_handler() _NOEXCEPT { -return __sync_fetch_and_add(&__new_handler, nullptr); +return __libcpp_atomic_load(&__new_handler); } } // namespace std Index: src/support/runtime/exception_fallback.ipp === --- src/support/runtime/exception_fallback.ipp +++ src/support/runtime/exception_fallback.ipp @@ -20,13 +20,13 @@ unexpected_handler set_unexpected(unexpected_handler func) _NOEXCEPT { - return __sync_lock_test_and_set(&__unexpected_handler, func); + return __libcpp_atomic_exchange(&__unexpected_handler, func); } unexpected_handler get_unexpected() _NOEXCEPT { - return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0); + return __libcpp_atomic_load(&__unexpected_handler); } @@ -41,14 +41,13 @@ terminate_handler set_terminate(terminate_handler func) _NOEXCEPT { - return __sync_lock_test_and_set(&__terminate_handler, func); + return __libcpp_atomic_exchange(&__terminate_handler, func); } terminate_handler get_terminate() _NOEXCEPT { - return __sync_fetch_and_add(&__terminate_handler, (terminate_handler)0); - + return __libcpp_atomic_load(&__terminate_handler); } #ifndef __EMSCRIPTEN__ // We provide this in JS Index: src/new.cpp === --- src/new.cpp +++ src/new.cpp @@ -12,6 +12,7 @@ #include #include "new" +#include "include/atomic_support.h" #if defined(_LIBCPP_ABI_MICROSOFT) // nothing todo Index: src/locale.cpp === --- src/locale.cpp +++ src/locale.cpp @@ -36,6 +36,7 @@ #endif #include #include +#include "include/atomic_support.h" #include "__undef_macros" // On Linux, wint_t and wchar_t have different signed-ness, and this causes @@ -667,7 +668,7 @@ void locale::id::__init() { -__id_ = __sync_add_and_fetch(&__next_id, 1); +__id_ = __libcpp_atomic_add(&__next_id, 1); } // template <> class collate_byname Index: src/include/refstring.h === --- src/include/refstring.h +++ src/include/refstring.h @@ -18,6 +18,7 @@ #include #include #endif +#include "atomic_support.h" _LIBCPP_BEGIN_NAMESPACE_STD @@ -83,19 +84,19 @@ : __imp_(s.__imp_) { if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); } inline __libcpp_refstring& __libcpp_refstring::operator=(__libcpp_refstring const& s) _NOEXCEPT { bool adjust_old_count = __uses_refcount(); struct _Rep_base *old_rep = rep_from_data(__imp_); __imp_ = s.__imp_; if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); if (adjust_old_count) { -if (__sync_add_and_fetch(&old_rep->count, count_t(-1)) < 0) +if (__libcpp_atomic_add(&old_rep->count, count_t(-1)) < 0) { ::operator delete(old_rep); } @@ -107,7 +108,7 @@ __libcpp_refstring::~__libcpp_refstring() { if (__uses_refcount()) { _Rep_base* rep = rep_from_data(__imp_); -if (__sync_add_and_fetch(&rep->count, count_t(-1)) < 0) { +if (__libcpp_atomic_add(&rep->count, count_t(-1)) < 0) { ::operator delete(rep); } } Index: src/include/atomic_support.h === --- src/include/atomic_support.h +++ src/include/atomic_support.h @@ -16,6 +16,7 @@ #if defined(__clang__) && __has_builtin(__atomic_load_n) \ && __has_builtin(__atomic_store_n)\ && __has_builtin(__atomic_add_fetch) \ + && __has_builtin(__atomic_exchange_n) \ && __has_builtin(__atomic_compare_exchange_n) \ && defined(__ATOMIC_RELAXED) \ && defined(__ATOMIC_CONSUME) \ @@ -82,6 +83,14 @@ return __atomic_add_fetch(__val, __a, __order); } +te
[PATCH] D38059: Rename list::base to list::__base.
dlj created this revision. Herald added a subscriber: sanjoy. Even though the base typedef is private, it still participates in lookup. For example: void base() {} struct X : private std::list { X() { base(); } }; https://reviews.llvm.org/D38059 Files: include/list Index: include/list === --- include/list +++ include/list @@ -805,30 +805,30 @@ class _LIBCPP_TEMPLATE_VIS list : private __list_imp<_Tp, _Alloc> { -typedef __list_imp<_Tp, _Alloc> base; -typedef typename base::__node __node; -typedef typename base::__node_allocator__node_allocator; -typedef typename base::__node_pointer __node_pointer; -typedef typename base::__node_alloc_traits __node_alloc_traits; -typedef typename base::__node_base __node_base; -typedef typename base::__node_base_pointer __node_base_pointer; -typedef typename base::__link_pointer __link_pointer; +typedef __list_imp<_Tp, _Alloc> __base; +typedef typename __base::__node __node; +typedef typename __base::__node_allocator__node_allocator; +typedef typename __base::__node_pointer __node_pointer; +typedef typename __base::__node_alloc_traits __node_alloc_traits; +typedef typename __base::__node_base __node_base; +typedef typename __base::__node_base_pointer __node_base_pointer; +typedef typename __base::__link_pointer __link_pointer; public: typedef _Tp value_type; typedef _Alloc allocator_type; static_assert((is_same::value), "Invalid allocator::value_type"); typedef value_type& reference; typedef const value_type&const_reference; -typedef typename base::pointer pointer; -typedef typename base::const_pointer const_pointer; -typedef typename base::size_type size_type; -typedef typename base::difference_type difference_type; -typedef typename base::iterator iterator; -typedef typename base::const_iteratorconst_iterator; -typedef _VSTD::reverse_iterator reverse_iterator; -typedef _VSTD::reverse_iterator const_reverse_iterator; +typedef typename __base::pointer pointer; +typedef typename __base::const_pointer const_pointer; +typedef typename __base::size_type size_type; +typedef typename __base::difference_type difference_type; +typedef typename __base::iteratoriterator; +typedef typename __base::const_iterator const_iterator; +typedef _VSTD::reverse_iteratorreverse_iterator; +typedef _VSTD::reverse_iterator const_reverse_iterator; _LIBCPP_INLINE_VISIBILITY list() @@ -839,7 +839,7 @@ #endif } _LIBCPP_INLINE_VISIBILITY -explicit list(const allocator_type& __a) : base(__a) +explicit list(const allocator_type& __a) : __base(__a) { #if _LIBCPP_DEBUG_LEVEL >= 2 __get_db()->__insert_c(this); @@ -895,29 +895,29 @@ allocator_type get_allocator() const _NOEXCEPT; _LIBCPP_INLINE_VISIBILITY -size_type size() const _NOEXCEPT {return base::__sz();} +size_type size() const _NOEXCEPT {return __base::__sz();} _LIBCPP_INLINE_VISIBILITY -bool empty() const _NOEXCEPT {return base::empty();} +bool empty() const _NOEXCEPT {return __base::empty();} _LIBCPP_INLINE_VISIBILITY size_type max_size() const _NOEXCEPT { return std::min( -base::__node_alloc_max_size(), +__base::__node_alloc_max_size(), numeric_limits::max()); } _LIBCPP_INLINE_VISIBILITY - iterator begin() _NOEXCEPT{return base::begin();} + iterator begin() _NOEXCEPT{return __base::begin();} _LIBCPP_INLINE_VISIBILITY -const_iterator begin() const _NOEXCEPT {return base::begin();} +const_iterator begin() const _NOEXCEPT {return __base::begin();} _LIBCPP_INLINE_VISIBILITY - iterator end() _NOEXCEPT {return base::end();} + iterator end() _NOEXCEPT {return __base::end();} _LIBCPP_INLINE_VISIBILITY -const_iterator end()const _NOEXCEPT {return base::end();} +const_iterator end()const _NOEXCEPT {return __base::end();} _LIBCPP_INLINE_VISIBILITY -const_iterator cbegin() const _NOEXCEPT {return base::begin();} +const_iterator cbegin() const _NOEXCEPT {return __base::begin();} _LIBCPP_INLINE_VISIBILITY -const_iterator cend() const _NOEXCEPT {return base::end();} +const_iterator cend() const _NOEXCEPT {return __base::end();} _LIBCPP_INLINE_VISIBILITY reverse_iterator rbegin() _NOEXCEPT @@
[libcxx] r313694 - [libc++] Replace __sync_* functions with __libcpp_atomic_* functions
Author: weimingz Date: Tue Sep 19 16:18:03 2017 New Revision: 313694 URL: http://llvm.org/viewvc/llvm-project?rev=313694&view=rev Log: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions Summary: This patch replaces __sync_* with __libcpp_atomic_* and adds a wrapper function for __atomic_exchange to support _LIBCPP_HAS_NO_THREADS. Reviewers: EricWF, jroelofs, mclow.lists, compnerd Reviewed By: EricWF, compnerd Subscribers: compnerd, efriedma, cfe-commits, joerg, llvm-commits Differential Revision: https://reviews.llvm.org/D35235 Modified: libcxx/trunk/src/exception.cpp libcxx/trunk/src/include/atomic_support.h libcxx/trunk/src/include/refstring.h libcxx/trunk/src/locale.cpp libcxx/trunk/src/new.cpp libcxx/trunk/src/support/runtime/exception_fallback.ipp libcxx/trunk/src/support/runtime/new_handler_fallback.ipp Modified: libcxx/trunk/src/exception.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/exception.cpp?rev=313694&r1=313693&r2=313694&view=diff == --- libcxx/trunk/src/exception.cpp (original) +++ libcxx/trunk/src/exception.cpp Tue Sep 19 16:18:03 2017 @@ -31,6 +31,7 @@ #include "support/runtime/exception_glibcxx.ipp" #include "support/runtime/exception_pointer_glibcxx.ipp" #else +#include "include/atomic_support.h" #include "support/runtime/exception_fallback.ipp" #include "support/runtime/exception_pointer_unimplemented.ipp" #endif Modified: libcxx/trunk/src/include/atomic_support.h URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/include/atomic_support.h?rev=313694&r1=313693&r2=313694&view=diff == --- libcxx/trunk/src/include/atomic_support.h (original) +++ libcxx/trunk/src/include/atomic_support.h Tue Sep 19 16:18:03 2017 @@ -16,6 +16,7 @@ #if defined(__clang__) && __has_builtin(__atomic_load_n) \ && __has_builtin(__atomic_store_n)\ && __has_builtin(__atomic_add_fetch) \ + && __has_builtin(__atomic_exchange_n) \ && __has_builtin(__atomic_compare_exchange_n) \ && defined(__ATOMIC_RELAXED) \ && defined(__ATOMIC_CONSUME) \ @@ -84,6 +85,14 @@ _ValueType __libcpp_atomic_add(_ValueTyp template inline _LIBCPP_INLINE_VISIBILITY +_ValueType __libcpp_atomic_exchange(_ValueType* __target, +_ValueType __value, int __order = _AO_Seq) +{ +return __atomic_exchange_n(__target, __value, __order); +} + +template +inline _LIBCPP_INLINE_VISIBILITY bool __libcpp_atomic_compare_exchange(_ValueType* __val, _ValueType* __expected, _ValueType __after, int __success_order = _AO_Seq, @@ -136,6 +145,16 @@ _ValueType __libcpp_atomic_add(_ValueTyp } template +inline _LIBCPP_INLINE_VISIBILITY +_ValueType __libcpp_atomic_exchange(_ValueType* __target, +_ValueType __value, int __order = _AO_Seq) +{ +_ValueType old = *__target; +*__target = __value; +return old; +} + +template inline _LIBCPP_INLINE_VISIBILITY bool __libcpp_atomic_compare_exchange(_ValueType* __val, _ValueType* __expected, _ValueType __after, Modified: libcxx/trunk/src/include/refstring.h URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/include/refstring.h?rev=313694&r1=313693&r2=313694&view=diff == --- libcxx/trunk/src/include/refstring.h (original) +++ libcxx/trunk/src/include/refstring.h Tue Sep 19 16:18:03 2017 @@ -18,6 +18,7 @@ #include #include #endif +#include "atomic_support.h" _LIBCPP_BEGIN_NAMESPACE_STD @@ -83,7 +84,7 @@ __libcpp_refstring::__libcpp_refstring(c : __imp_(s.__imp_) { if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); } inline @@ -92,10 +93,10 @@ __libcpp_refstring& __libcpp_refstring:: struct _Rep_base *old_rep = rep_from_data(__imp_); __imp_ = s.__imp_; if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); if (adjust_old_count) { -if (__sync_add_and_fetch(&old_rep->count, count_t(-1)) < 0) +if (__libcpp_atomic_add(&old_rep->count, count_t(-1)) < 0) { ::operator delete(old_rep); } @@ -107,7 +108,7 @@ inline __libcpp_refstring::~__libcpp_refstring() { if (__uses_refcount()) { _Rep_base* rep = rep_from_data(__imp_); -if (__sync_add_and_fetch(&rep->count, count_t(-1)) < 0) { +if (__libcpp_atomic_add(&rep->count, count_t(-1)) < 0) { ::operator delete(rep); }
[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions
This revision was automatically updated to reflect the committed changes. Closed by commit rL313694: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions (authored by weimingz). Changed prior to commit: https://reviews.llvm.org/D35235?vs=115918&id=115922#toc Repository: rL LLVM https://reviews.llvm.org/D35235 Files: libcxx/trunk/src/exception.cpp libcxx/trunk/src/include/atomic_support.h libcxx/trunk/src/include/refstring.h libcxx/trunk/src/locale.cpp libcxx/trunk/src/new.cpp libcxx/trunk/src/support/runtime/exception_fallback.ipp libcxx/trunk/src/support/runtime/new_handler_fallback.ipp Index: libcxx/trunk/src/support/runtime/new_handler_fallback.ipp === --- libcxx/trunk/src/support/runtime/new_handler_fallback.ipp +++ libcxx/trunk/src/support/runtime/new_handler_fallback.ipp @@ -15,13 +15,13 @@ new_handler set_new_handler(new_handler handler) _NOEXCEPT { -return __sync_lock_test_and_set(&__new_handler, handler); +return __libcpp_atomic_exchange(&__new_handler, handler); } new_handler get_new_handler() _NOEXCEPT { -return __sync_fetch_and_add(&__new_handler, nullptr); +return __libcpp_atomic_load(&__new_handler); } } // namespace std Index: libcxx/trunk/src/support/runtime/exception_fallback.ipp === --- libcxx/trunk/src/support/runtime/exception_fallback.ipp +++ libcxx/trunk/src/support/runtime/exception_fallback.ipp @@ -20,13 +20,13 @@ unexpected_handler set_unexpected(unexpected_handler func) _NOEXCEPT { - return __sync_lock_test_and_set(&__unexpected_handler, func); + return __libcpp_atomic_exchange(&__unexpected_handler, func); } unexpected_handler get_unexpected() _NOEXCEPT { - return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0); + return __libcpp_atomic_load(&__unexpected_handler); } @@ -41,14 +41,13 @@ terminate_handler set_terminate(terminate_handler func) _NOEXCEPT { - return __sync_lock_test_and_set(&__terminate_handler, func); + return __libcpp_atomic_exchange(&__terminate_handler, func); } terminate_handler get_terminate() _NOEXCEPT { - return __sync_fetch_and_add(&__terminate_handler, (terminate_handler)0); - + return __libcpp_atomic_load(&__terminate_handler); } #ifndef __EMSCRIPTEN__ // We provide this in JS Index: libcxx/trunk/src/include/atomic_support.h === --- libcxx/trunk/src/include/atomic_support.h +++ libcxx/trunk/src/include/atomic_support.h @@ -16,6 +16,7 @@ #if defined(__clang__) && __has_builtin(__atomic_load_n) \ && __has_builtin(__atomic_store_n)\ && __has_builtin(__atomic_add_fetch) \ + && __has_builtin(__atomic_exchange_n) \ && __has_builtin(__atomic_compare_exchange_n) \ && defined(__ATOMIC_RELAXED) \ && defined(__ATOMIC_CONSUME) \ @@ -84,6 +85,14 @@ template inline _LIBCPP_INLINE_VISIBILITY +_ValueType __libcpp_atomic_exchange(_ValueType* __target, +_ValueType __value, int __order = _AO_Seq) +{ +return __atomic_exchange_n(__target, __value, __order); +} + +template +inline _LIBCPP_INLINE_VISIBILITY bool __libcpp_atomic_compare_exchange(_ValueType* __val, _ValueType* __expected, _ValueType __after, int __success_order = _AO_Seq, @@ -137,6 +146,16 @@ template inline _LIBCPP_INLINE_VISIBILITY +_ValueType __libcpp_atomic_exchange(_ValueType* __target, +_ValueType __value, int __order = _AO_Seq) +{ +_ValueType old = *__target; +*__target = __value; +return old; +} + +template +inline _LIBCPP_INLINE_VISIBILITY bool __libcpp_atomic_compare_exchange(_ValueType* __val, _ValueType* __expected, _ValueType __after, int = 0, int = 0) Index: libcxx/trunk/src/include/refstring.h === --- libcxx/trunk/src/include/refstring.h +++ libcxx/trunk/src/include/refstring.h @@ -18,6 +18,7 @@ #include #include #endif +#include "atomic_support.h" _LIBCPP_BEGIN_NAMESPACE_STD @@ -83,19 +84,19 @@ : __imp_(s.__imp_) { if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); } inline __libcpp_refstring& __libcpp_refstring::operator=(__libcpp_refstring const& s) _NOEXCEPT { bool adjust_old_count = __uses_refcount(); struct _Rep_base *old_rep = rep_from_data(__imp_); __imp_ = s.__imp_; if (__uses_refcount()) -__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1); +__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1); if (adjust_old_count)