[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker
peter.smith added a comment. Thanks for pointing that out, I'm out of office today, will look at describing the intention to fall through when I get back in on Monday. Comment at: lib/Driver/ToolChains/Gnu.cpp:241 + case llvm::Triple::thumbeb: +IsBigEndian = true; + case llvm::Triple::arm: xbolva00 wrote: > Gnu.cpp:241:17: warning: this statement may fall through > [-Wimplicit-fallthrough=] > IsBigEndian = true; The fall through is intentional in this case. Repository: rC Clang https://reviews.llvm.org/D52784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53417: [Clang][PowerPC] Choose a better candidate as function call if there is a compatible vector conversion instead of ambious call error
wuzish created this revision. wuzish added reviewers: hfinkel, nemanjai, Douglasgido, hubert.reinterpretcast, fpichet. There are 2 function variations with altivec type parameter. When we call them with argument of generic gcc vector type we would prefer to choose the variation with implicit argument conversion of compatible vector type. typedef float __v4sf __attribute__((__vector_size__(16))); void f(vector float); void f(vector signed int); int main { __v4sf a; f(a); } Here, we'd like to choose f(vector float) but not report an ambiguous call error. Repository: rC Clang https://reviews.llvm.org/D53417 Files: clang/lib/Sema/SemaOverload.cpp clang/test/Sema/altivec-generic-overload.c Index: clang/test/Sema/altivec-generic-overload.c === --- /dev/null +++ clang/test/Sema/altivec-generic-overload.c @@ -0,0 +1,101 @@ +// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only + +typedef signed char __v4sc __attribute__((__vector_size__(16))); +typedef unsigned char __v4uc __attribute__((__vector_size__(16))); +typedef signed short __v4ss __attribute__((__vector_size__(16))); +typedef unsigned short __v4us __attribute__((__vector_size__(16))); +typedef signed int __v4si __attribute__((__vector_size__(16))); +typedef unsigned int __v4ui __attribute__((__vector_size__(16))); +typedef signed long long __v4sll __attribute__((__vector_size__(16))); +typedef unsigned long long __v4ull __attribute__((__vector_size__(16))); +typedef signed __int128 __v4slll __attribute__((__vector_size__(16))); +typedef unsigned __int128 __v4ulll __attribute__((__vector_size__(16))); +typedef float __v4f __attribute__((__vector_size__(16))); +typedef double __v4d __attribute__((__vector_size__(16))); + +void __attribute__((__overloadable__)) convert1(vector signed char); +void __attribute__((__overloadable__)) convert1(vector unsigned char); +void __attribute__((__overloadable__)) convert1(vector signed short); +void __attribute__((__overloadable__)) convert1(vector unsigned short); +void __attribute__((__overloadable__)) convert1(vector signed int); +void __attribute__((__overloadable__)) convert1(vector unsigned int); +void __attribute__((__overloadable__)) convert1(vector signed long long); +void __attribute__((__overloadable__)) convert1(vector unsigned long long); +void __attribute__((__overloadable__)) convert1(vector signed __int128); +void __attribute__((__overloadable__)) convert1(vector unsigned __int128); +void __attribute__((__overloadable__)) convert1(vector float); +void __attribute__((__overloadable__)) convert1(vector double); +void __attribute__((__overloadable__)) convert1(vector bool int); +void __attribute__((__overloadable__)) convert1(vector pixel short); +void __attribute__((__overloadable__)) convert2(__v4sc); +void __attribute__((__overloadable__)) convert2(__v4uc); +void __attribute__((__overloadable__)) convert2(__v4ss); +void __attribute__((__overloadable__)) convert2(__v4us); +void __attribute__((__overloadable__)) convert2(__v4si); +void __attribute__((__overloadable__)) convert2(__v4ui); +void __attribute__((__overloadable__)) convert2(__v4sll); +void __attribute__((__overloadable__)) convert2(__v4ull); +void __attribute__((__overloadable__)) convert2(__v4slll); +void __attribute__((__overloadable__)) convert2(__v4ulll); +void __attribute__((__overloadable__)) convert2(__v4f); +void __attribute__((__overloadable__)) convert2(__v4d); + +void test() +{ + __v4sc gv1; + __v4uc gv2; + __v4ss gv3; + __v4us gv4; + __v4si gv5; + __v4ui gv6; + __v4sll gv7; + __v4ull gv8; + __v4slll gv9; + __v4ulll gv10; + __v4f gv11; + __v4d gv12; + + vector signed char av1; + vector unsigned char av2; + vector signed short av3; + vector unsigned short av4; + vector signed int av5; + vector unsigned int av6; + vector signed long long av7; + vector unsigned long long av8; + vector signed __int128 av9; + vector unsigned __int128 av10; + vector float av11; + vector double av12; + vector bool int av13; + vector pixel short av14; + + convert1(gv1); + convert1(gv2); + convert1(gv3); + convert1(gv4); + convert1(gv5); + convert1(gv6); + convert1(gv7); + convert1(gv8); + convert1(gv9); + convert1(gv10); + convert1(gv11); + convert1(gv12); + + convert2(av1); + convert2(av2); + convert2(av3); + convert2(av4); + convert2(av5); + convert2(av6); + convert2(av7); + convert2(av8); + convert2(av9); + convert2(av10); + convert2(av11); + convert2(av12); + convert2(av13); // expected-error {{call to 'convert2' is ambiguous}} + convert2(av14); // expected-error {{call to 'convert2' is ambiguous}} + +} Index: clang/lib/Sema/SemaOverload.cpp === --- clang/lib/Sema/SemaOverload.cpp +++ clang/lib/Sema/SemaOverload.cpp @@ -3900,6 +3
[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension
sidorovd added a comment. Ping https://reviews.llvm.org/D51402 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker
xbolva00 added inline comments. Comment at: lib/Driver/ToolChains/Gnu.cpp:241 + case llvm::Triple::thumbeb: +IsBigEndian = true; + case llvm::Triple::arm: peter.smith wrote: > xbolva00 wrote: > > Gnu.cpp:241:17: warning: this statement may fall through > > [-Wimplicit-fallthrough=] > > IsBigEndian = true; > The fall through is intentional in this case. Please mark it with LLVM_FALLTHROUGH then. Repository: rC Clang https://reviews.llvm.org/D52784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambious call error
wuzish added a comment. If anybody knows who are familiar with C/C++ function overload and code related to this issue, please feel free to add them as reviewers and subscribers. Repository: rC Clang https://reviews.llvm.org/D53417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
curdeius updated this revision to Diff 170152. curdeius added a comment. Fixed diff. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 Files: clang-tidy/readability/ElseAfterReturnCheck.cpp test/clang-tidy/readability-else-after-return-if-constexpr.cpp Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp === --- /dev/null +++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp @@ -0,0 +1,22 @@ +// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++17 + +// Constexpr if is an exception to the rule, we cannot remove the else. +void f() { + if (sizeof(int) > 4) +return; + else +return; + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return' + + if constexpr (sizeof(int) > 4) +return; + else +return; + + if constexpr (sizeof(int) > 4) +return; + else if constexpr (sizeof(long) > 4) +return; + else +return; +} Index: clang-tidy/readability/ElseAfterReturnCheck.cpp === --- clang-tidy/readability/ElseAfterReturnCheck.cpp +++ clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -25,7 +25,8 @@ expr(ignoringImplicit(cxxThrowExpr().bind("throw"); Finder->addMatcher( compoundStmt(forEach( - ifStmt(hasThen(stmt( + ifStmt(unless(isConstexpr()), + hasThen(stmt( anyOf(ControlFlowInterruptorMatcher, compoundStmt(has(ControlFlowInterruptorMatcher), hasElse(stmt().bind("else"))) Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp === --- /dev/null +++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp @@ -0,0 +1,22 @@ +// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++17 + +// Constexpr if is an exception to the rule, we cannot remove the else. +void f() { + if (sizeof(int) > 4) +return; + else +return; + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return' + + if constexpr (sizeof(int) > 4) +return; + else +return; + + if constexpr (sizeof(int) > 4) +return; + else if constexpr (sizeof(long) > 4) +return; + else +return; +} Index: clang-tidy/readability/ElseAfterReturnCheck.cpp === --- clang-tidy/readability/ElseAfterReturnCheck.cpp +++ clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -25,7 +25,8 @@ expr(ignoringImplicit(cxxThrowExpr().bind("throw"); Finder->addMatcher( compoundStmt(forEach( - ifStmt(hasThen(stmt( + ifStmt(unless(isConstexpr()), + hasThen(stmt( anyOf(ControlFlowInterruptorMatcher, compoundStmt(has(ControlFlowInterruptorMatcher), hasElse(stmt().bind("else"))) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
sammccall added a comment. Thanks! @arphaman, any concerns about the change to extension format? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53391 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
sammccall updated this revision to Diff 170153. sammccall marked an inline comment as done. sammccall added a comment. Don't call URIForFile again, rebase. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53391 Files: clangd/ClangdLSPServer.cpp clangd/Diagnostics.cpp clangd/Diagnostics.h clangd/Protocol.cpp clangd/Protocol.h test/clangd/fixits-embed-in-diagnostic.test unittests/clangd/ClangdUnitTests.cpp Index: unittests/clangd/ClangdUnitTests.cpp === --- unittests/clangd/ClangdUnitTests.cpp +++ unittests/clangd/ClangdUnitTests.cpp @@ -199,11 +199,13 @@ // Transform dianostics and check the results. std::vector>> LSPDiags; - toLSPDiags(D, [&](clangd::Diagnostic LSPDiag, -llvm::ArrayRef Fixes) { -LSPDiags.push_back({std::move(LSPDiag), -std::vector(Fixes.begin(), Fixes.end())}); - }); + toLSPDiags( + D, URIForFile("/path/to/foo/bar/main.cpp"), ClangdDiagnosticOptions(), + [&](clangd::Diagnostic LSPDiag, llvm::ArrayRef Fixes) { +LSPDiags.push_back( +{std::move(LSPDiag), + std::vector(Fixes.begin(), Fixes.end())}); + }); EXPECT_THAT( LSPDiags, Index: test/clangd/fixits-embed-in-diagnostic.test === --- test/clangd/fixits-embed-in-diagnostic.test +++ test/clangd/fixits-embed-in-diagnostic.test @@ -1,12 +1,12 @@ # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true}}},"trace":"off"}} +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"codeActionsInline":true}}},"trace":"off"}} --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}} -# CHECK:"method": "textDocument/publishDiagnostics", -# CHECK-NEXT:"params": { -# CHECK-NEXT: "diagnostics": [ +# CHECK: "method": "textDocument/publishDiagnostics", +# CHECK-NEXT: "params": { +# CHECK-NEXT:"diagnostics": [ # CHECK-NEXT: { -# CHECK-NEXT:"clangd_fixes": [ +# CHECK-NEXT:"codeActions": [ # CHECK-NEXT: { # CHECK-NEXT:"edit": { # CHECK-NEXT: "changes": { @@ -27,6 +27,7 @@ # CHECK-NEXT:] # CHECK-NEXT: } # CHECK-NEXT:}, +# CHECK-NEXT:"kind": "quickfix", # CHECK-NEXT:"title": "change 'union' to 'struct'" # CHECK-NEXT: } # CHECK-NEXT:], Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -323,9 +323,8 @@ /// workspace.symbol.symbolKind.valueSet llvm::Optional WorkspaceSymbolKinds; - /// Whether the client accepts diagnostics with fixes attached using the - /// "clangd_fixes" extension. - /// textDocument.publishDiagnostics.clangdFixSupport + /// Whether the client accepts diagnostics with codeActions attached inline. + /// textDocument.publishDiagnostics.codeActionsInline. bool DiagnosticFixes = false; /// Whether the client accepts diagnostics with category attached to it @@ -536,6 +535,7 @@ }; bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &); +struct CodeAction; struct Diagnostic { /// The range at which the message applies. Range range; @@ -560,7 +560,12 @@ /// An LSP extension that's used to send the name of the category over to the /// client. The category typically describes the compilation stage during /// which the issue was produced, e.g. "Semantic Issue" or "Parse Issue". - std::string category; + llvm::Optional category; + + /// Clangd extension: code actions related to this diagnostic. + /// Only with capability textDocument.publishDiagnostics.codeActionsInline. + /// (These actions can also be obtained using textDocument/codeAction). + llvm::Optional> codeActions; }; llvm::json::Value toJSON(const Diagnostic &); Index: clangd/Protocol.cpp === --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -210,8 +210,8 @@ if (auto *Diagnostics = TextDocument->getObject("publishDiagnostics")) { if (auto CategorySupport = Diagnostics->getBoolean("categorySupport")) R.DiagnosticCategory = *CategorySupport; - if (auto ClangdFixSupport = Diagnostics->getBoolean("clangdFixSupport")) -R.DiagnosticFixes = *ClangdFixSupport; + if (auto CodeActions = Diagnostics->getBoolean("codeActionsInline")) +R.DiagnosticFixes = *CodeActions; } if (auto *Completion = TextDocument->getObject("
[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines
yvvan added a comment. Do you know the better way to accomplish my aim than adding an option to libFormat? For example making a dependent library which serves a little different purpose than libFormat itself or something simpler? https://reviews.llvm.org/D53072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension
AlexeySotkin added a comment. Ping https://reviews.llvm.org/D51484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. ping? Can I go forward in this way? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
hokein updated this revision to Diff 170157. hokein marked an inline comment as done. hokein added a comment. Log the whole location when overflow happens. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53400 Files: clangd/XRefs.cpp clangd/index/Index.cpp clangd/index/Index.h unittests/clangd/IndexTests.cpp Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -46,10 +46,15 @@ EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } Index: clangd/index/Index.h === --- clangd/index/Index.h +++ clangd/index/Index.h @@ -42,6 +42,10 @@ void setColumn(uint32_t Column); uint32_t column() const { return Column; } +bool hasOverflow() const { + return Line >= MaxLine || Column >= MaxColumn; +} + static constexpr uint32_t MaxLine = (1 << 20) - 1; static constexpr uint32_t MaxColumn = (1 << 12) - 1; Index: clangd/index/Index.cpp === --- clangd/index/Index.cpp +++ clangd/index/Index.cpp @@ -23,15 +23,13 @@ constexpr uint32_t SymbolLocation::Position::MaxColumn; void SymbolLocation::Position::setLine(uint32_t L) { if (L > MaxLine) { -log("Set an overflowed Line {0}", L); Line = MaxLine; return; } Line = L; } void SymbolLocation::Position::setColumn(uint32_t Col) { if (Col > MaxColumn) { -log("Set an overflowed Column {0}", Col); Column = MaxColumn; return; } Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -37,6 +37,11 @@ return nullptr; } +void logIfOverflow(const SymbolLocation Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ LSPLoc.range.start.character = Loc.Start.column(); LSPLoc.range.end.line = Loc.End.line(); LSPLoc.range.end.character = Loc.End.column(); + logIfOverflow(Loc); return LSPLoc; } Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -46,10 +46,15 @@ EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } Index: clangd/index/Index.h === --- clangd/index/Index.h +++ clangd/index/Index.h @@ -42,6 +42,10 @@ void setColumn(uint32_t Column); uint32_t column() const { return Column; } +bool hasOverflow() const { + return Line >= MaxLine || Column >= MaxColumn; +} + static constexpr uint32_t MaxLine = (1 << 20) - 1; static constexpr uint32_t MaxColumn = (1 << 12) - 1; Index: clangd/index/Index.cpp === --- clangd/index/Index.cpp +++ clangd/index/Index.cpp @@ -23,15 +23,13 @@ constexpr uint32_t SymbolLocation::Position::MaxColumn; void SymbolLocation::Position::setLine(uint32_t L) { if (L > MaxLine) { -log("Set an overflowed Line {0}", L); Line = MaxLine; return; } Line = L; } void SymbolLocation::Position::setColumn(uint32_t Col) { if (Col > MaxColumn) { -log("Set an overflowed Column {0}", Col); Column = MaxColumn; return; } Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -37,6 +37,11 @@ return nullptr; } +void logIfOverflow(const SymbolLocation Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ LSPLoc.rang
[PATCH] D53400: [clangd] Remove the overflow log.
hokein added inline comments. Comment at: clangd/XRefs.cpp:43 + if (Line >= SymbolLocation::Position::MaxLine) +log("Get an overflowed line"); + return Line; sammccall wrote: > Log message could use more context. And I think it'd be really useful to > print the whole location here. > > bikeshed: you could add a method `bool Position::hasOverflow()`, and then > write below > > ``` > if (LSPLoc.range.start.hasOverflow() || LSPLoc.range.end.hasOverflow()) > log("Possible overflow in symbol location: {0}", LSPLoc); > ``` > > Distinguishing between line/column overflow isn't important if you're > printing the full range anyway. This is neat. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53400 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
hokein updated this revision to Diff 170159. hokein added a comment. Minor cleanup. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53400 Files: clangd/XRefs.cpp clangd/index/Index.cpp clangd/index/Index.h unittests/clangd/IndexTests.cpp Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -46,10 +46,15 @@ EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } Index: clangd/index/Index.h === --- clangd/index/Index.h +++ clangd/index/Index.h @@ -43,6 +43,10 @@ void setColumn(uint32_t Column); uint32_t column() const { return Column; } +bool hasOverflow() const { + return Line >= MaxLine || Column >= MaxColumn; +} + static constexpr uint32_t MaxLine = (1 << 20) - 1; static constexpr uint32_t MaxColumn = (1 << 12) - 1; Index: clangd/index/Index.cpp === --- clangd/index/Index.cpp +++ clangd/index/Index.cpp @@ -23,15 +23,13 @@ constexpr uint32_t SymbolLocation::Position::MaxColumn; void SymbolLocation::Position::setLine(uint32_t L) { if (L > MaxLine) { -log("Set an overflowed Line {0}", L); Line = MaxLine; return; } Line = L; } void SymbolLocation::Position::setColumn(uint32_t Col) { if (Col > MaxColumn) { -log("Set an overflowed Column {0}", Col); Column = MaxColumn; return; } Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -37,6 +37,11 @@ return nullptr; } +void logIfOverflow(const SymbolLocation &Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ LSPLoc.range.start.character = Loc.Start.column(); LSPLoc.range.end.line = Loc.End.line(); LSPLoc.range.end.character = Loc.End.column(); + logIfOverflow(Loc); return LSPLoc; } Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -46,10 +46,15 @@ EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } Index: clangd/index/Index.h === --- clangd/index/Index.h +++ clangd/index/Index.h @@ -43,6 +43,10 @@ void setColumn(uint32_t Column); uint32_t column() const { return Column; } +bool hasOverflow() const { + return Line >= MaxLine || Column >= MaxColumn; +} + static constexpr uint32_t MaxLine = (1 << 20) - 1; static constexpr uint32_t MaxColumn = (1 << 12) - 1; Index: clangd/index/Index.cpp === --- clangd/index/Index.cpp +++ clangd/index/Index.cpp @@ -23,15 +23,13 @@ constexpr uint32_t SymbolLocation::Position::MaxColumn; void SymbolLocation::Position::setLine(uint32_t L) { if (L > MaxLine) { -log("Set an overflowed Line {0}", L); Line = MaxLine; return; } Line = L; } void SymbolLocation::Position::setColumn(uint32_t Col) { if (Col > MaxColumn) { -log("Set an overflowed Column {0}", Col); Column = MaxColumn; return; } Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -37,6 +37,11 @@ return nullptr; } +void logIfOverflow(const SymbolLocation &Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ LSPLoc.range.start.character = Loc.Start.column(); LSPLoc.range.end.line = Loc
[clang-tools-extra] r344777 - [clangd] Remove the overflow log.
Author: hokein Date: Fri Oct 19 01:35:24 2018 New Revision: 344777 URL: http://llvm.org/viewvc/llvm-project?rev=344777&view=rev Log: [clangd] Remove the overflow log. Summary: LLVM codebase has generated files (all are build/Target/XXX/*.inc) that exceed the MaxLine & MaxColumn. Printing these log would be noisy. Reviewers: sammccall Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits Differential Revision: https://reviews.llvm.org/D53400 Modified: clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/clangd/index/Index.cpp clang-tools-extra/trunk/clangd/index/Index.h clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Modified: clang-tools-extra/trunk/clangd/XRefs.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=344777&r1=344776&r2=344777&view=diff == --- clang-tools-extra/trunk/clangd/XRefs.cpp (original) +++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Oct 19 01:35:24 2018 @@ -37,6 +37,11 @@ const Decl *getDefinition(const Decl *D) return nullptr; } +void logIfOverflow(const SymbolLocation &Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ llvm::Optional toLSPLocation(c LSPLoc.range.start.character = Loc.Start.column(); LSPLoc.range.end.line = Loc.End.line(); LSPLoc.range.end.character = Loc.End.column(); + logIfOverflow(Loc); return LSPLoc; } Modified: clang-tools-extra/trunk/clangd/index/Index.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=344777&r1=344776&r2=344777&view=diff == --- clang-tools-extra/trunk/clangd/index/Index.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Index.cpp Fri Oct 19 01:35:24 2018 @@ -23,7 +23,6 @@ constexpr uint32_t SymbolLocation::Posit constexpr uint32_t SymbolLocation::Position::MaxColumn; void SymbolLocation::Position::setLine(uint32_t L) { if (L > MaxLine) { -log("Set an overflowed Line {0}", L); Line = MaxLine; return; } @@ -31,7 +30,6 @@ void SymbolLocation::Position::setLine(u } void SymbolLocation::Position::setColumn(uint32_t Col) { if (Col > MaxColumn) { -log("Set an overflowed Column {0}", Col); Column = MaxColumn; return; } Modified: clang-tools-extra/trunk/clangd/index/Index.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=344777&r1=344776&r2=344777&view=diff == --- clang-tools-extra/trunk/clangd/index/Index.h (original) +++ clang-tools-extra/trunk/clangd/index/Index.h Fri Oct 19 01:35:24 2018 @@ -43,6 +43,10 @@ struct SymbolLocation { void setColumn(uint32_t Column); uint32_t column() const { return Column; } +bool hasOverflow() const { + return Line >= MaxLine || Column >= MaxColumn; +} + static constexpr uint32_t MaxLine = (1 << 20) - 1; static constexpr uint32_t MaxColumn = (1 << 12) - 1; Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp?rev=344777&r1=344776&r2=344777&view=diff == --- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Fri Oct 19 01:35:24 2018 @@ -46,10 +46,15 @@ TEST(SymbolLocation, Position) { EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
This revision was automatically updated to reflect the committed changes. Closed by commit rL344777: [clangd] Remove the overflow log. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53400 Files: clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/clangd/index/Index.cpp clang-tools-extra/trunk/clangd/index/Index.h clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp @@ -46,10 +46,15 @@ EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } Index: clang-tools-extra/trunk/clangd/XRefs.cpp === --- clang-tools-extra/trunk/clangd/XRefs.cpp +++ clang-tools-extra/trunk/clangd/XRefs.cpp @@ -37,6 +37,11 @@ return nullptr; } +void logIfOverflow(const SymbolLocation &Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ LSPLoc.range.start.character = Loc.Start.column(); LSPLoc.range.end.line = Loc.End.line(); LSPLoc.range.end.character = Loc.End.column(); + logIfOverflow(Loc); return LSPLoc; } Index: clang-tools-extra/trunk/clangd/index/Index.h === --- clang-tools-extra/trunk/clangd/index/Index.h +++ clang-tools-extra/trunk/clangd/index/Index.h @@ -43,6 +43,10 @@ void setColumn(uint32_t Column); uint32_t column() const { return Column; } +bool hasOverflow() const { + return Line >= MaxLine || Column >= MaxColumn; +} + static constexpr uint32_t MaxLine = (1 << 20) - 1; static constexpr uint32_t MaxColumn = (1 << 12) - 1; Index: clang-tools-extra/trunk/clangd/index/Index.cpp === --- clang-tools-extra/trunk/clangd/index/Index.cpp +++ clang-tools-extra/trunk/clangd/index/Index.cpp @@ -23,15 +23,13 @@ constexpr uint32_t SymbolLocation::Position::MaxColumn; void SymbolLocation::Position::setLine(uint32_t L) { if (L > MaxLine) { -log("Set an overflowed Line {0}", L); Line = MaxLine; return; } Line = L; } void SymbolLocation::Position::setColumn(uint32_t Col) { if (Col > MaxColumn) { -log("Set an overflowed Column {0}", Col); Column = MaxColumn; return; } Index: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp @@ -46,10 +46,15 @@ EXPECT_EQ(1u, Pos.line()); Pos.setColumn(2); EXPECT_EQ(2u, Pos.column()); + EXPECT_FALSE(Pos.hasOverflow()); Pos.setLine(Position::MaxLine + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.line(), Position::MaxLine); + Pos.setLine(1); // reset the overflowed line. + Pos.setColumn(Position::MaxColumn + 1); // overflow + EXPECT_TRUE(Pos.hasOverflow()); EXPECT_EQ(Pos.column(), Position::MaxColumn); } Index: clang-tools-extra/trunk/clangd/XRefs.cpp === --- clang-tools-extra/trunk/clangd/XRefs.cpp +++ clang-tools-extra/trunk/clangd/XRefs.cpp @@ -37,6 +37,11 @@ return nullptr; } +void logIfOverflow(const SymbolLocation &Loc) { + if (Loc.Start.hasOverflow() || Loc.End.hasOverflow()) +log("Possible overflow in symbol location: {0}", Loc); +} + // Convert a SymbolLocation to LSP's Location. // HintPath is used to resolve the path of URI. // FIXME: figure out a good home for it, and share the implementation with @@ -61,6 +66,7 @@ LSPLoc.range.start.character = Loc.Start.column(); LSPLoc.range.end.line = Loc.End.line(); LSPLoc.range.end.character = Loc.End.column(); + logIfOverflow(Loc); return LSPLoc; } Index: clang-tools-extra/trunk/clangd/index/Index.h === --- clang-tools-extra/trunk/clangd/index/Index.h +++ clang-tools-extra/trunk/clangd/index/Index.h @@ -43,6 +43,10 @@ void setColumn(uint32_t Column); uint32_t co
r344778 - [OpenCL] Remove unwanted signedness conversion from tests
Author: mantognini Date: Fri Oct 19 02:01:37 2018 New Revision: 344778 URL: http://llvm.org/viewvc/llvm-project?rev=344778&view=rev Log: [OpenCL] Remove unwanted signedness conversion from tests The get_kernel_* functions used in cl20-device-side-enqueue.cl all return unsigned integers. This patch avoids undesired implicit conversions on the returned values. Differential Revision: https://reviews.llvm.org/D52873 Modified: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl Modified: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl?rev=344778&r1=344777&r2=344778&view=diff == --- cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl (original) +++ cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl Fri Oct 19 02:01:37 2018 @@ -212,7 +212,7 @@ kernel void work_group_size_tests() { #pragma OPENCL EXTENSION cl_khr_subgroups : enable -kernel void foo(global int *buf) +kernel void foo(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); @@ -220,7 +220,7 @@ kernel void foo(global int *buf) buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}} } -kernel void bar(global int *buf) +kernel void bar(global unsigned int *buf) { __private ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); @@ -230,13 +230,13 @@ kernel void bar(global int *buf) #pragma OPENCL EXTENSION cl_khr_subgroups : disable -kernel void foo1(global int *buf) +kernel void foo1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}} } -kernel void bar1(global int *buf) +kernel void bar1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52873: Remove unwanted signedness conversion from tests
This revision was automatically updated to reflect the committed changes. Closed by commit rC344778: [OpenCL] Remove unwanted signedness conversion from tests (authored by mantognini, committed by ). Repository: rC Clang https://reviews.llvm.org/D52873 Files: test/SemaOpenCL/cl20-device-side-enqueue.cl Index: test/SemaOpenCL/cl20-device-side-enqueue.cl === --- test/SemaOpenCL/cl20-device-side-enqueue.cl +++ test/SemaOpenCL/cl20-device-side-enqueue.cl @@ -212,15 +212,15 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : enable -kernel void foo(global int *buf) +kernel void foo(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}} buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}} } -kernel void bar(global int *buf) +kernel void bar(global unsigned int *buf) { __private ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); @@ -230,13 +230,13 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : disable -kernel void foo1(global int *buf) +kernel void foo1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}} } -kernel void bar1(global int *buf) +kernel void bar1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}} Index: test/SemaOpenCL/cl20-device-side-enqueue.cl === --- test/SemaOpenCL/cl20-device-side-enqueue.cl +++ test/SemaOpenCL/cl20-device-side-enqueue.cl @@ -212,15 +212,15 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : enable -kernel void foo(global int *buf) +kernel void foo(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}} buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}} } -kernel void bar(global int *buf) +kernel void bar(global unsigned int *buf) { __private ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); @@ -230,13 +230,13 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : disable -kernel void foo1(global int *buf) +kernel void foo1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}} } -kernel void bar1(global int *buf) +kernel void bar1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52873: Remove unwanted signedness conversion from tests
This revision was automatically updated to reflect the committed changes. Closed by commit rL344778: [OpenCL] Remove unwanted signedness conversion from tests (authored by mantognini, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52873?vs=168256&id=170165#toc Repository: rL LLVM https://reviews.llvm.org/D52873 Files: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl Index: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl === --- cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl +++ cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl @@ -212,15 +212,15 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : enable -kernel void foo(global int *buf) +kernel void foo(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}} buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}} } -kernel void bar(global int *buf) +kernel void bar(global unsigned int *buf) { __private ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); @@ -230,13 +230,13 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : disable -kernel void foo1(global int *buf) +kernel void foo1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}} } -kernel void bar1(global int *buf) +kernel void bar1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}} Index: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl === --- cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl +++ cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl @@ -212,15 +212,15 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : enable -kernel void foo(global int *buf) +kernel void foo(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}} buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}} } -kernel void bar(global int *buf) +kernel void bar(global unsigned int *buf) { __private ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); @@ -230,13 +230,13 @@ #pragma OPENCL EXTENSION cl_khr_subgroups : disable -kernel void foo1(global int *buf) +kernel void foo1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}} } -kernel void bar1(global int *buf) +kernel void bar1(global unsigned int *buf) { ndrange_t n; buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53369: [CodeComplete] Fix accessibility of protected members when accessing members implicitly.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. The change looks reasonable to me, I'd let @ilya-biryukov take a second look. Repository: rC Clang https://reviews.llvm.org/D53369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51633: [ASTImporter] Added error handling for AST import.
balazske added inline comments. Comment at: lib/AST/ASTImporter.cpp:8197 -void ASTImporter::ImportDefinition(Decl *From) { +Error ASTImporter::ImportDefinition_New(Decl *From) { Decl *To = Import(From); a_sidorin wrote: > ImportDefinitionOrError? The intention was to make the `_New` name only temporary until only this new function is used therefore it is not a nice name (requires changes in LLDB). Then it should be renamed back to `ImportDefinition`. It is probably not needed to indicate in the name that the function can fail, it can be seen from the return value. (This will be the case with other `Import` functions later.) Repository: rC Clang https://reviews.llvm.org/D51633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53397: [MinGW] Link to correct openmp library
mstorsjo added a comment. Please add a testcase in test/Driver/, it looks like fopenmp.c there might be a good target to add a few lines in. Repository: rC Clang https://reviews.llvm.org/D53397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
donat.nagy updated this revision to Diff 170169. donat.nagy added a comment. Give a reference for the significand size values of the IEEE 754 floating point types; cleanup comments and formatting. Repository: rC Clang https://reviews.llvm.org/D52730 Files: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp test/Analysis/conversion.c Index: test/Analysis/conversion.c === --- test/Analysis/conversion.c +++ test/Analysis/conversion.c @@ -137,6 +137,12 @@ U8 = S + 10; } +char dontwarn6(long long x) { + long long y = 42; + y += x; + return y == 42; +} + // C library functions, handled via apiModeling.StdCLibraryFunctions @@ -154,7 +160,7 @@ # define EOF (-1) char reply_string[8192]; FILE *cin; -extern int dostuff (void); +extern int dostuff(void); int libraryFunction2() { int c, n; int dig; @@ -179,3 +185,26 @@ } } +double floating_point(long long a, int b) { + if (a > 1LL << 55) { +double r = a; // expected-warning {{Loss of precision}} +return r; + } else if (b > 1 << 25) { +float f = b; // expected-warning {{Loss of precision}} +return f; + } + return 137; +} + +double floating_point2() { + int a = 1 << 24; + long long b = 1LL << 53; + double d = a; // no-warning + double f = b; // no-warning + return d - f; +} + +int floating_point_3(unsigned long long a) { + double b = a; // no-warning + return 42; +} Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp === --- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -14,8 +14,10 @@ // of expressions. A warning is reported when: // * a negative value is implicitly converted to an unsigned value in an // assignment, comparison or multiplication. -// * assignment / initialization when source value is greater than the max -// value of target +// * assignment / initialization when the source value is greater than the max +// value of the target integer type +// * assignment / initialization when the source integer is above the range +// where the target floating point type can represent all integers // // Many compilers and tools have similar checks that are based on semantic // analysis. Those checks are sound but have poor precision. ConversionChecker @@ -29,6 +31,8 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include + using namespace clang; using namespace ento; @@ -40,11 +44,9 @@ private: mutable std::unique_ptr BT; - // Is there loss of precision bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType, CheckerContext &C) const; - // Is there loss of sign bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const; void reportBug(ExplodedNode *N, CheckerContext &C, const char Msg[]) const; @@ -132,19 +134,72 @@ QualType SubType = Cast->IgnoreParenImpCasts()->getType(); - if (!DestType->isIntegerType() || !SubType->isIntegerType()) + if (!DestType->isRealType() || !SubType->isIntegerType()) return false; - if (C.getASTContext().getIntWidth(DestType) >= - C.getASTContext().getIntWidth(SubType)) + const bool isInteger = DestType->isIntegerType(); + + const auto &AC = C.getASTContext(); + + // We will find the largest RepresentsUntilExp value such that the DestType + // can exactly represent all nonnegative integers below 2^RepresentsUntilExp. + unsigned RepresentsUntilExp; + + if (isInteger) { +RepresentsUntilExp = AC.getIntWidth(DestType); +if (RepresentsUntilExp == 1) { + // This is just casting a number to bool, probably not a bug. + return false; +} +if (DestType->isSignedIntegerType()) + RepresentsUntilExp--; + } else { +unsigned FloatingSize = AC.getTypeSize(DestType); + +// We assume that we are dealing with IEEE 754 floating point types +// (including e.g. the 80-bit long double). The constants in the following +// code come from the standard, see e.g.: https://enwp.org/IEEE_754 + +switch (FloatingSize) { + case 64: +// double (and possibly long double on some systems) +RepresentsUntilExp = 53; +break; + case 32: +// float +RepresentsUntilExp = 24; +break; + case 16: +// _Float16 +RepresentsUntilExp = 12; +break; + default: +// A larger type, which can represent all integers below 2^64. +return false; +} + } + + if (RepresentsUntilExp >= sizeof(unsigned long long) * CHAR_BIT) { +// Avoid overflow in our later calculations. return false; + } - unsigned W = C.getASTContext().getIntWidth(DestType); - if (W == 1 || W >= 64U) + unsigned CorrectedSrcWidth = AC.getIntWidth(SubType); + if (SubType->isSignedIntegerType()) +Cor
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
donat.nagy marked 2 inline comments as done. donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164 +// double and possibly long double on some systems +RepresentsUntilExp = 53; break; + case 32: donat.nagy wrote: > xazax.hun wrote: > > A link to the source of these number would be useful. How are these > > calculated. Also, as far as I know the current C++ standard does not > > require anything about how floating points are represented in an > > implementation. So it would be great to somehow refer to the representation > > used by clang rather than hardcoding these values. (Note that I am > > perfectly fine with warning for implementation defined behavior as the > > original version also warn for such in case of integers.) > I took these magic numbers from the IEEE 754 standard; I completely agree > that their introduction is far from being elegant. > > Unfortunately it seems that referring to the representation used by clang > seems to be somewhat difficult, see e.g. this old [[ > https://stackoverflow.com/questions/13780931/how-do-i-get-llvm-types-from-clang > | stackoverflow answer ]]. In the Z3 solver a similar problem was solved by > defining a static function ([[ > https://clang.llvm.org/doxygen/Z3ConstraintManager_8cpp_source.html#l00269 | > getFloatSemantics() ]]) which uses a switch to translate the bit width of a > floating point type into an llvm::fltSemantics value (which contains the > precision value as a field). > > I could imagine three solutions: > > - reimplementing the logic getFloatSemantics, > - moving getFloatSemantics to some utility library and using it from there, > - keeping the current code, with comments describing my assumptions and > referencing the IEEE standard. > > Which of these is the best? > > Note: According to the documentation [[ > https://releases.llvm.org/2.5/docs/LangRef.html#t_floating | the floating > point types ]] supported by the LLVM IR are just float, double and some high > precision extension types (which are handled by the `default:` branch of my > code). Unfortunately, I do not know what happens to the [[ > https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point > | `_Float16` ]] half-width float type. I updated the patch with a comment describing my assumptions, but I will implement a different solution if that would be better. Repository: rC Clang https://reviews.llvm.org/D52730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. This would save us 8 bytes per ref, and buy us ~50MB in total for llvm index (from ~350MB to ~300 MB). The char pointer must be null-terminated, and llvm::StringSaver guarantees it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53427 Files: clangd/CodeComplete.cpp clangd/FindSymbols.cpp clangd/Quality.cpp clangd/XRefs.cpp clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp clangd/index/Serialization.cpp clangd/index/SymbolCollector.cpp clangd/index/YAMLSerialization.cpp clangd/index/dex/Dex.cpp clangd/index/dex/dexp/Dexp.cpp unittests/clangd/CodeCompleteTests.cpp unittests/clangd/DexTests.cpp unittests/clangd/FileIndexTests.cpp unittests/clangd/IndexTests.cpp unittests/clangd/SerializationTests.cpp unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp === --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -53,8 +53,8 @@ return (arg.Name + arg.CompletionSnippetSuffix).str() == S; } MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } -MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; } -MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; } +MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.fileURI() == P; } +MATCHER_P(DefURI, P, "") { return arg.Definition.fileURI() == P; } MATCHER_P(IncludeHeader, P, "") { return (arg.IncludeHeaders.size() == 1) && (arg.IncludeHeaders.begin()->IncludeHeader == P); Index: unittests/clangd/SerializationTests.cpp === --- unittests/clangd/SerializationTests.cpp +++ unittests/clangd/SerializationTests.cpp @@ -110,7 +110,7 @@ EXPECT_EQ(Sym1.Signature, ""); EXPECT_EQ(Sym1.Documentation, "Foo doc"); EXPECT_EQ(Sym1.ReturnType, "int"); - EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h"); + EXPECT_EQ(Sym1.CanonicalDeclaration.fileURI(), "file:///path/foo.h"); EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static); EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); @@ -121,7 +121,7 @@ EXPECT_THAT(Sym2, QName("clang::Foo2")); EXPECT_EQ(Sym2.Signature, "-sig"); EXPECT_EQ(Sym2.ReturnType, ""); - EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h"); + EXPECT_EQ(Sym2.CanonicalDeclaration.fileURI(), "file:///path/bar.h"); EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion); EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated); @@ -133,7 +133,7 @@ testing::SizeIs(1; auto Ref1 = ParsedYAML->Refs->begin()->second.front(); EXPECT_EQ(Ref1.Kind, RefKind::Reference); - EXPECT_EQ(Ref1.Location.FileURI, "file:///path/foo.cc"); + EXPECT_EQ(Ref1.Location.fileURI(), "file:///path/foo.cc"); } std::vector YAMLFromSymbols(const SymbolSlab &Slab) { Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -36,7 +36,7 @@ std::make_tuple(Range.start.line, Range.start.character, Range.end.line, Range.end.character); } -MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; } +MATCHER_P(FileURI, F, "") { return arg.Location.fileURI() == F; } TEST(SymbolLocation, Position) { using Position = SymbolLocation::Position; @@ -206,8 +206,8 @@ Symbol L, R; L.ID = R.ID = SymbolID("hello"); L.Name = R.Name = "Foo"; // same in both - L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs - R.CanonicalDeclaration.FileURI = "file:///right.h"; + L.CanonicalDeclaration.setFileURI("file:///left.h"); // differs + R.CanonicalDeclaration.setFileURI("file:///right.h"); L.References = 1; R.References = 2; L.Signature = "()"; // present in left only @@ -218,7 +218,7 @@ Symbol M = mergeSymbol(L, R); EXPECT_EQ(M.Name, "Foo"); - EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h"); + EXPECT_EQ(M.CanonicalDeclaration.fileURI(), "file:///left.h"); EXPECT_EQ(M.References, 3u); EXPECT_EQ(M.Signature, "()"); EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}"); @@ -231,20 +231,20 @@ Symbol L, R; L.ID = R.ID = SymbolID("hello"); - L.CanonicalDeclaration.FileURI = "file:/left.h"; - R.CanonicalDeclaration.FileURI = "file:/right.h"; + L.CanonicalDeclaration.setFileURI("file:/left.h"); + R.CanonicalDeclaration.setFileURI("file:/right.h"); L.Name = "left"; R.Name = "right"; Symbol M = mergeSymbol(L, R); - EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h"); - EXPECT_EQ(M.Definition.Fil
[PATCH] D53428: Adjust cl::opt uses for opt_storage refactored
yrouban created this revision. yrouban added reviewers: chandlerc, rsmith, reames, skatkov. Herald added a subscriber: cfe-commits. This patch fixes compilation errors that result from refactoring of LLVM cl::opt structures introduced by https://reviews.llvm.org/D53426. Basically for class-based cl::opt to access to the instance of MyClass one of the overloadable operators should be used. For example if we have an option defined as cl::opt MyStringOption; Then the changes should look like the following: Old: MyStringOption.c_str() New: MyStringOption->c_str() Old: cout << MyStringOption; New: cout << *MyStringOption; Repository: rC Clang https://reviews.llvm.org/D53428 Files: lib/Tooling/Execution.cpp tools/arcmt-test/arcmt-test.cpp tools/clang-import-test/clang-import-test.cpp tools/clang-rename/ClangRename.cpp tools/diagtool/FindDiagnosticID.cpp unittests/Tooling/ExecutionTest.cpp Index: unittests/Tooling/ExecutionTest.cpp === --- unittests/Tooling/ExecutionTest.cpp +++ unittests/Tooling/ExecutionTest.cpp @@ -158,7 +158,7 @@ auto Executor = internal::createExecutorFromCommandLineArgsImpl( argc, &argv[0], TestCategory); ASSERT_TRUE((bool)Executor); - EXPECT_EQ(BeforeReset, "set"); + EXPECT_EQ(*BeforeReset, "set"); BeforeReset.removeArgument(); } Index: tools/diagtool/FindDiagnosticID.cpp === --- tools/diagtool/FindDiagnosticID.cpp +++ tools/diagtool/FindDiagnosticID.cpp @@ -66,7 +66,7 @@ return 0; } -llvm::errs() << "error: invalid diagnostic '" << DiagnosticName << "'\n"; +llvm::errs() << "error: invalid diagnostic '" << *DiagnosticName << "'\n"; return 1; } OS << Diag->DiagID << "\n"; Index: tools/clang-rename/ClangRename.cpp === --- tools/clang-rename/ClangRename.cpp +++ tools/clang-rename/ClangRename.cpp @@ -104,9 +104,9 @@ if (!Input.empty()) { // Populate QualifiedNames and NewNames from a YAML file. ErrorOr> Buffer = -llvm::MemoryBuffer::getFile(Input); +llvm::MemoryBuffer::getFile(*Input); if (!Buffer) { - errs() << "clang-rename: failed to read " << Input << ": " + errs() << "clang-rename: failed to read " << *Input << ": " << Buffer.getError().message() << "\n"; return 1; } Index: tools/clang-import-test/clang-import-test.cpp === --- tools/clang-import-test/clang-import-test.cpp +++ tools/clang-import-test/clang-import-test.cpp @@ -174,7 +174,7 @@ { using namespace driver::types; -ID Id = lookupTypeForTypeSpecifier(Input.c_str()); +ID Id = lookupTypeForTypeSpecifier(Input->c_str()); assert(Id != TY_INVALID); if (isCXX(Id)) { Inv->getLangOpts()->CPlusPlus = true; Index: tools/arcmt-test/arcmt-test.cpp === --- tools/arcmt-test/arcmt-test.cpp +++ tools/arcmt-test/arcmt-test.cpp @@ -243,7 +243,7 @@ if (RemappingsFile.empty()) inputBuf = MemoryBuffer::getSTDIN(); else -inputBuf = MemoryBuffer::getFile(RemappingsFile); +inputBuf = MemoryBuffer::getFile(*RemappingsFile); if (!inputBuf) { errs() << "error: could not read remappings input\n"; return true; Index: lib/Tooling/Execution.cpp === --- lib/Tooling/Execution.cpp +++ lib/Tooling/Execution.cpp @@ -82,7 +82,7 @@ return std::move(*Executor); } return llvm::make_error( - llvm::Twine("Executor \"") + ExecutorName + "\" is not registered.", + llvm::Twine("Executor \"") + *ExecutorName + "\" is not registered.", llvm::inconvertibleErrorCode()); } } // end namespace internal Index: unittests/Tooling/ExecutionTest.cpp === --- unittests/Tooling/ExecutionTest.cpp +++ unittests/Tooling/ExecutionTest.cpp @@ -158,7 +158,7 @@ auto Executor = internal::createExecutorFromCommandLineArgsImpl( argc, &argv[0], TestCategory); ASSERT_TRUE((bool)Executor); - EXPECT_EQ(BeforeReset, "set"); + EXPECT_EQ(*BeforeReset, "set"); BeforeReset.removeArgument(); } Index: tools/diagtool/FindDiagnosticID.cpp === --- tools/diagtool/FindDiagnosticID.cpp +++ tools/diagtool/FindDiagnosticID.cpp @@ -66,7 +66,7 @@ return 0; } -llvm::errs() << "error: invalid diagnostic '" << DiagnosticName << "'\n"; +llvm::errs() << "error: invalid diagnostic '" << *DiagnosticName << "'\n"; return 1; } OS << Diag->DiagID << "\n"; Index: tools/clang-rename/ClangRename.cpp === --- tools/clang-rename/ClangRename.cpp +++ tools/clang-rename/ClangR
[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:182 + // - if there were multiple replies, only the first is sent + class ReplyOnce { +struct State { As discussed offline, we could potentially make this non-copyable to make harder to be duplicated and called more than once. And we could also get rid of the shared `State` in the functor. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change
joerg added a comment. Excuse me for bring this up so late, but why do we want to make any such promises? As in: fundamentally, LLVM IR doesn't have any order property on the module level. I have yet so seen reasonable code where the order of functions matters for anything but performance. I've seen a few things that required `-funit-at-a-time`, most noticable GCC's CRT implementation. But those are all major hacks. So under what premise is it useful to have to even pretend to honor source code order? Repository: rC Clang https://reviews.llvm.org/D34796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:69 + // Macros are ignored. + if (Arg->getBeginLoc().isMacroID()) +return; hwright wrote: > JonasToth wrote: > > maybe `assert` instead, as your comment above suggests that macros are > > already filtered out? > This is a different check than above. > > In the first case, we want to be sure we aren't replacing cases inside of a > macro, such as: > ``` > #define SECONDS(x) absl::Seconds(x) > SECONDS(1.0) > ``` > > In this one, we want to avoid changing the argument if it is itself a macro: > ``` > #define VAL 1.0 > absl::Seconds(VAL); > ``` > > So it is a separate check, not just a re-assertion of the first one. Ok, I misunderstood the code then :) Comment at: test/clang-tidy/abseil-duration-factory-float.cpp:32 +void ConvertFloatTest() { + absl::Duration d; + hwright wrote: > JonasToth wrote: > > Could you also provide test cases with hexadecimal floating literals, which > > are C++17? The thousand-separators could be checked as well (dunno the > > correct term right now, but the `1'000'000` feature). > > Please add test cases for negative literals, too. > Added the hexadecimal floating literal tests. > > The testing infrastructure wouldn't build the test source with `3'000` as a > literal argument. (There's also an argument that by the time we get to the > AST, that distinction is gone anyway and this test isn't the place to check > comprehensive literal parsing.) > > I've also added a negative literal test, to illustrate that we don't yet > handle that case (which is in practice pretty rare). I'd rather add it in a > separate change. I am happy with that. I agree that parsing should deal with it fine and your code seems to do it fine as well. My experience is, that sometimes there are still surprises :) https://reviews.llvm.org/D53339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53433: [clangd] *Prototype* auto-index stores symbols per-file instead of per-TU.
ioeric created this revision. ioeric added reviewers: sammccall, hokein. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This allows us to deduplicate header symbols across TUs. File digests are collects when collecting symbols/refs. And the index store deduplicates file symbols based on the file digest. WIP: This is still a prototype and apparently needs better names and more tests. Looking for early feedback to make sure this is heading the right direction. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/IndexAction.cpp clangd/index/IndexAction.h clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h clangd/indexer/IndexerMain.cpp unittests/clangd/BackgroundIndexTests.cpp unittests/clangd/FileIndexTests.cpp unittests/clangd/SyncAPI.cpp unittests/clangd/SyncAPI.h Index: unittests/clangd/SyncAPI.h === --- unittests/clangd/SyncAPI.h +++ unittests/clangd/SyncAPI.h @@ -52,6 +52,7 @@ SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query); SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req); +RefSlab getRefs(const SymbolIndex &Index, SymbolID ID); } // namespace clangd } // namespace clang Index: unittests/clangd/SyncAPI.cpp === --- unittests/clangd/SyncAPI.cpp +++ unittests/clangd/SyncAPI.cpp @@ -8,6 +8,7 @@ //===--===// #include "SyncAPI.h" +#include "index/Index.h" namespace clang { namespace clangd { @@ -137,5 +138,14 @@ return std::move(Builder).build(); } +RefSlab getRefs(const SymbolIndex &Index, SymbolID ID) { + RefsRequest Req; + Req.IDs = {ID}; + RefSlab::Builder Slab; + Index.refs(Req, [&](const Ref &S) { Slab.insert(ID, S); }); + return std::move(Slab).build(); +} + + } // namespace clangd } // namespace clang Index: unittests/clangd/FileIndexTests.cpp === --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -72,14 +72,6 @@ return llvm::make_unique(std::move(Slab).build()); } -RefSlab getRefs(const SymbolIndex &I, SymbolID ID) { - RefsRequest Req; - Req.IDs = {ID}; - RefSlab::Builder Slab; - I.refs(Req, [&](const Ref &S) { Slab.insert(ID, S); }); - return std::move(Slab).build(); -} - TEST(FileSymbolsTest, UpdateAndGet) { FileSymbols FS; EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""), IsEmpty()); Index: unittests/clangd/BackgroundIndexTests.cpp === --- unittests/clangd/BackgroundIndexTests.cpp +++ unittests/clangd/BackgroundIndexTests.cpp @@ -1,36 +1,78 @@ #include "SyncAPI.h" #include "TestFS.h" #include "index/Background.h" +#include "gmock/gmock-generated-matchers.h" +#include "gmock/gmock-matchers.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +using testing::_; +using testing::AllOf; +using testing::Not; using testing::UnorderedElementsAre; namespace clang { namespace clangd { MATCHER_P(Named, N, "") { return arg.Name == N; } +MATCHER(Declared, "") { return !arg.CanonicalDeclaration.FileURI.empty(); } +MATCHER(Defined, "") { return !arg.Definition.FileURI.empty(); } + +MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; } +testing::Matcher +RefsAre(std::vector> Matchers) { + return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers))); +} TEST(BackgroundIndexTest, IndexTwoFiles) { MockFSProvider FS; // a.h yields different symbols when included by A.cc vs B.cc. // Currently we store symbols for each TU, so we get both. - FS.Files[testPath("root/A.h")] = "void a_h(); void NAME(){}"; - FS.Files[testPath("root/A.cc")] = "#include \"A.h\""; - FS.Files[testPath("root/B.cc")] = "#define NAME bar\n#include \"A.h\""; + FS.Files[testPath("root/A.h")] = R"( + void common(); + void f_b(); + #if CC == A +class A_H {}; + #elif CC == B +class B_H {}; + #else +class _H {}; + #endif + )"; + FS.Files[testPath("root/A.cc")] = + "#include \"A.h\"\nvoid g() { (void)common; }"; + FS.Files[testPath("root/B.cc")] = + "#define CC B\n#include \"A.h\"\nvoid f_b() { (void)common; }"; BackgroundIndex Idx(Context::empty(), "", FS); tooling::CompileCommand Cmd; Cmd.Filename = testPath("root/A.cc"); Cmd.Directory = testPath("root"); - Cmd.CommandLine = {"clang++", "-DNAME=foo", testPath("root/A.cc")}; + Cmd.CommandLine = {"clang++", "-DCC=A", testPath("root/A.cc")}; Idx.enqueue(testPath("root"), Cmd); - Cmd.CommandLine.back() = Cmd.Filename = testPath("root/B.cc"); + + Idx.blockUntilIdleForTest();
[PATCH] D53434: Java annotation declaration being handled correctly
SamMaier created this revision. Herald added a subscriber: cfe-commits. Previously, Java annotation declarations (`@interface AnnotationName`) were being handled as ObjC interfaces. This caused the brace formatting to mess up, so that when you had a class with an interface defined in it, it would indent the final brace of the class. It used to format this class like so: class A { @interface B {} } But will now just skip the @interface and format it like so: class A { @interface B {} } Repository: rC Clang https://reviews.llvm.org/D53434 Files: lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTestJava.cpp Index: unittests/Format/FormatTestJava.cpp === --- unittests/Format/FormatTestJava.cpp +++ unittests/Format/FormatTestJava.cpp @@ -155,6 +155,15 @@ " void doStuff(int theStuff);\n" " void doMoreStuff(int moreStuff);\n" "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {\n" + "int stuff;\n" + "void doMoreStuff(int moreStuff);\n" + " }\n" + "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {}\n" + "}"); } TEST_F(FormatTestJava, AnonymousClasses) { Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -1130,6 +1130,10 @@ nextToken(); parseBracedList(); break; + } else if (Style.Language == FormatStyle::LK_Java && + FormatTok->is(Keywords.kw_interface)) { +nextToken(); +break; } switch (FormatTok->Tok.getObjCKeywordID()) { case tok::objc_public: Index: unittests/Format/FormatTestJava.cpp === --- unittests/Format/FormatTestJava.cpp +++ unittests/Format/FormatTestJava.cpp @@ -155,6 +155,15 @@ " void doStuff(int theStuff);\n" " void doMoreStuff(int moreStuff);\n" "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {\n" + "int stuff;\n" + "void doMoreStuff(int moreStuff);\n" + " }\n" + "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {}\n" + "}"); } TEST_F(FormatTestJava, AnonymousClasses) { Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -1130,6 +1130,10 @@ nextToken(); parseBracedList(); break; + } else if (Style.Language == FormatStyle::LK_Java && + FormatTok->is(Keywords.kw_interface)) { +nextToken(); +break; } switch (FormatTok->Tok.getObjCKeywordID()) { case tok::objc_public: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:3908 + QualType ToType) { +assert(FromType->isVectorType() && "FromType should be a vector type"); +assert(ToType->isVectorType() && "ToType should be a vector type"); These `assert`s would be redundant if `castAs` is used instead of `getAs`. Comment at: clang/lib/Sema/SemaOverload.cpp:3911 + +if (FromType->getAs()->getVectorKind() == +VectorType::AltiVecVector || Use `castAs` here and below. Comment at: clang/lib/Sema/SemaOverload.cpp:3930 + // } + // Here, we'd like to choose f(vector float) but not + // report an ambiguous call error s/but/and/; Repository: rC Clang https://reviews.llvm.org/D53417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344784 - [ASTImporter] Removed uneeded default case label.
Author: balazske Date: Fri Oct 19 08:16:51 2018 New Revision: 344784 URL: http://llvm.org/viewvc/llvm-project?rev=344784&view=rev Log: [ASTImporter] Removed uneeded default case label. Modified: cfe/trunk/lib/AST/ASTImporter.cpp Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=344784&r1=344783&r2=344784&view=diff == --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Oct 19 08:16:51 2018 @@ -89,9 +89,9 @@ namespace clang { return "UnsupportedConstruct"; case Unknown: return "Unknown error"; -default: - llvm_unreachable("Invalid error code."); } +llvm_unreachable("Invalid error code."); +return "Invalid error code."; } void ImportError::log(raw_ostream &OS) const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53433: [clangd] *Prototype* auto-index stores symbols per-file instead of per-TU.
sammccall added inline comments. Comment at: clangd/index/FileIndex.cpp:167 +} +// FIXME: aggregate symbol reference count based on references. +for (const auto &Sym : FileSyms.getValue()->Refs) { This is a good FIXME to carry over to FileSymbols::buildIndex(). It may (or may not) make sense to control this behavior with the same param that controls merge behavior, the logic being that it's also a quality/accuracy tradeoff. Comment at: clangd/index/FileIndex.h:49 +/// rename the existing FileSymbols to something else e.g. TUSymbols? +class SymbolsGroupedByFiles { +public: `FileSymbols` isn't actually that opinionated about the data it manages: - the keys ("filenames") just identify shards so we can tell whether a new shard is an addition or replaces an existing one. - the values (tu symbols/refs) are merged using pretty generic logic, we don't look at the details. I think we should be able to use `FileSymbols` mostly unmodified, and store digests in parallel, probably in `BackgroundIndexer`. Is there a strong reason to store "main file" digests separately to header digests? There are a couple of weak points: - The merging makes subtle assumptions: for symbols it picks one copy arbitrarily, assuming there are many copies (merging would be expensive) and they're mostly interchangeable duplicates. We could add a constructor or `buildIndex()` param to FileSymbols to control this, similar to the IndexType param. (For refs it assumes no duplicates and simply concatenates, which is also fine for our purposes). - `FileSymbols` locks internally to be threadsafe while keeping critical sections small. To keep digests in sync with FileSymbols contents we probably have to lock externally with a second mutex. This is a little ugly but not a real problem I think. Comment at: clangd/index/IndexAction.h:31 +std::function RefsCallback, +std::function FileDigestsCallback); these parallel callbacks that always get called together are a bit ridiculous (my fault). Can you add a FIXME to replace them with a single callback that passes a struct? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344785 - [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
Author: mkurdej Date: Fri Oct 19 08:26:17 2018 New Revision: 344785 URL: http://llvm.org/viewvc/llvm-project?rev=344785&view=rev Log: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if. Summary: It fixes the false positive when using constexpr if and where else cannot be removed: Example: ``` if constexpr (sizeof(int) > 4) // ... return /* ... */; else // This else cannot be removed. // ... return /* ... */; ``` Reviewers: alexfh, aaron.ballman Reviewed By: aaron.ballman Subscribers: lebedev.ri, xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D53372 Added: clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp?rev=344785&r1=344784&r2=344785&view=diff == --- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Fri Oct 19 08:26:17 2018 @@ -1,57 +1,58 @@ -//===--- ElseAfterReturnCheck.cpp - clang-tidy-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#include "ElseAfterReturnCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Tooling/FixIt.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace readability { - -void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { - const auto ControlFlowInterruptorMatcher = - stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"), - breakStmt().bind("break"), - expr(ignoringImplicit(cxxThrowExpr().bind("throw"); - Finder->addMatcher( - compoundStmt(forEach( - ifStmt(hasThen(stmt( - anyOf(ControlFlowInterruptorMatcher, - compoundStmt(has(ControlFlowInterruptorMatcher), - hasElse(stmt().bind("else"))) - .bind("if"))), - this); -} - -void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) { - const auto *If = Result.Nodes.getNodeAs("if"); - SourceLocation ElseLoc = If->getElseLoc(); - std::string ControlFlowInterruptor; - for (const auto *BindingName : {"return", "continue", "break", "throw"}) -if (Result.Nodes.getNodeAs(BindingName)) - ControlFlowInterruptor = BindingName; - - DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'") - << ControlFlowInterruptor; - Diag << tooling::fixit::createRemoval(ElseLoc); - - // FIXME: Removing the braces isn't always safe. Do a more careful analysis. - // FIXME: Change clang-format to correctly un-indent the code. - if (const auto *CS = Result.Nodes.getNodeAs("else")) -Diag << tooling::fixit::createRemoval(CS->getLBracLoc()) - << tooling::fixit::createRemoval(CS->getRBracLoc()); -} - -} // namespace readability -} // namespace tidy -} // namespace clang +//===--- ElseAfterReturnCheck.cpp - clang-tidy-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "ElseAfterReturnCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { + const auto ControlFlowInterruptorMatcher = + stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"), + breakStmt().bind("break"), + expr(ignoringImplicit(cxxThrowExpr().bind("throw"); + Finder->addMatcher( + compoundStmt(forEach( + ifStmt(unless(isConstexpr()), + hasThen(stmt( + anyOf(ControlFlowInterruptorMatcher, + compoundStmt(has(ControlFlowInterruptorMatcher), + hasElse(stmt().bind("else"))) + .bind("if"))), + this); +} + +void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) { + const auto *If = Result.Nodes.getNodeAs("if"); + SourceLocation ElseLoc = I
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE344785: [clang-tidy] Resolve readability-else-after-return false positive for constexpr… (authored by mkurdej, committed by ). Changed prior to commit: https://reviews.llvm.org/D53372?vs=170152&id=170201#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 Files: clang-tidy/readability/ElseAfterReturnCheck.cpp test/clang-tidy/readability-else-after-return-if-constexpr.cpp Index: clang-tidy/readability/ElseAfterReturnCheck.cpp === --- clang-tidy/readability/ElseAfterReturnCheck.cpp +++ clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -1,57 +1,58 @@ -//===--- ElseAfterReturnCheck.cpp - clang-tidy-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#include "ElseAfterReturnCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Tooling/FixIt.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace readability { - -void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { - const auto ControlFlowInterruptorMatcher = - stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"), - breakStmt().bind("break"), - expr(ignoringImplicit(cxxThrowExpr().bind("throw"); - Finder->addMatcher( - compoundStmt(forEach( - ifStmt(hasThen(stmt( - anyOf(ControlFlowInterruptorMatcher, - compoundStmt(has(ControlFlowInterruptorMatcher), - hasElse(stmt().bind("else"))) - .bind("if"))), - this); -} - -void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) { - const auto *If = Result.Nodes.getNodeAs("if"); - SourceLocation ElseLoc = If->getElseLoc(); - std::string ControlFlowInterruptor; - for (const auto *BindingName : {"return", "continue", "break", "throw"}) -if (Result.Nodes.getNodeAs(BindingName)) - ControlFlowInterruptor = BindingName; - - DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'") - << ControlFlowInterruptor; - Diag << tooling::fixit::createRemoval(ElseLoc); - - // FIXME: Removing the braces isn't always safe. Do a more careful analysis. - // FIXME: Change clang-format to correctly un-indent the code. - if (const auto *CS = Result.Nodes.getNodeAs("else")) -Diag << tooling::fixit::createRemoval(CS->getLBracLoc()) - << tooling::fixit::createRemoval(CS->getRBracLoc()); -} - -} // namespace readability -} // namespace tidy -} // namespace clang +//===--- ElseAfterReturnCheck.cpp - clang-tidy-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "ElseAfterReturnCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { + const auto ControlFlowInterruptorMatcher = + stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"), + breakStmt().bind("break"), + expr(ignoringImplicit(cxxThrowExpr().bind("throw"); + Finder->addMatcher( + compoundStmt(forEach( + ifStmt(unless(isConstexpr()), + hasThen(stmt( + anyOf(ControlFlowInterruptorMatcher, + compoundStmt(has(ControlFlowInterruptorMatcher), + hasElse(stmt().bind("else"))) + .bind("if"))), + this); +} + +void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) { + const auto *If = Result.Nodes.getNodeAs("if"); + SourceLocation ElseLoc = If->getElseLoc(); + std::string ControlFlowInterruptor; + for (const auto *BindingName : {"return", "continue", "break", "throw"}) +if (Result.Nodes.getNodeAs(BindingName)) + ControlFlowInterruptor = BindingName; + + DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'") + << ControlFlowInterruptor; + Diag << tooling::fixit::createRemoval(ElseLoc); + + // FIXME: Removing the braces isn't always safe. Do a more careful analysis. + // FIXME: Change clang-format to correctly un-indent the code. + if (const auto *CS = Result.Nodes.getNodeAs("else")
r344786 - [Hexagon] Remove support for V4
Author: kparzysz Date: Fri Oct 19 08:36:45 2018 New Revision: 344786 URL: http://llvm.org/viewvc/llvm-project?rev=344786&view=rev Log: [Hexagon] Remove support for V4 Modified: cfe/trunk/docs/ClangCommandLineReference.rst cfe/trunk/include/clang/Driver/Options.td cfe/trunk/lib/Basic/Targets/Hexagon.cpp cfe/trunk/test/Driver/hexagon-toolchain-elf.c cfe/trunk/test/Misc/target-invalid-cpu-note.c Modified: cfe/trunk/docs/ClangCommandLineReference.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=344786&r1=344785&r2=344786&view=diff == --- cfe/trunk/docs/ClangCommandLineReference.rst (original) +++ cfe/trunk/docs/ClangCommandLineReference.rst Fri Oct 19 08:36:45 2018 @@ -2146,7 +2146,7 @@ Link stack frames through backchain on S .. option:: -mconsole -.. option:: -mcpu=, -mv4 (equivalent to -mcpu=hexagonv4), -mv5 (equivalent to -mcpu=hexagonv5), -mv55 (equivalent to -mcpu=hexagonv55), -mv60 (equivalent to -mcpu=hexagonv60), -mv62 (equivalent to -mcpu=hexagonv62), -mv65 (equivalent to -mcpu=hexagonv65) +.. option:: -mcpu=, -mv5 (equivalent to -mcpu=hexagonv5), -mv55 (equivalent to -mcpu=hexagonv55), -mv60 (equivalent to -mcpu=hexagonv60), -mv62 (equivalent to -mcpu=hexagonv62), -mv65 (equivalent to -mcpu=hexagonv65) .. option:: -mcrc, -mno-crc Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=344786&r1=344785&r2=344786&view=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Fri Oct 19 08:36:45 2018 @@ -2661,8 +2661,6 @@ def _ : Joined<["--"], "">, Flags<[Unsup // Hexagon feature flags. def mieee_rnd_near : Flag<["-"], "mieee-rnd-near">, Group; -def mv4 : Flag<["-"], "mv4">, Group, - Alias, AliasArgs<["hexagonv4"]>; def mv5 : Flag<["-"], "mv5">, Group, Alias, AliasArgs<["hexagonv5"]>; def mv55 : Flag<["-"], "mv55">, Group, Modified: cfe/trunk/lib/Basic/Targets/Hexagon.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Hexagon.cpp?rev=344786&r1=344785&r2=344786&view=diff == --- cfe/trunk/lib/Basic/Targets/Hexagon.cpp (original) +++ cfe/trunk/lib/Basic/Targets/Hexagon.cpp Fri Oct 19 08:36:45 2018 @@ -25,14 +25,7 @@ void HexagonTargetInfo::getTargetDefines Builder.defineMacro("__qdsp6__", "1"); Builder.defineMacro("__hexagon__", "1"); - if (CPU == "hexagonv4") { -Builder.defineMacro("__HEXAGON_V4__"); -Builder.defineMacro("__HEXAGON_ARCH__", "4"); -if (Opts.HexagonQdsp6Compat) { - Builder.defineMacro("__QDSP6_V4__"); - Builder.defineMacro("__QDSP6_ARCH__", "4"); -} - } else if (CPU == "hexagonv5") { + if (CPU == "hexagonv5") { Builder.defineMacro("__HEXAGON_V5__"); Builder.defineMacro("__HEXAGON_ARCH__", "5"); if (Opts.HexagonQdsp6Compat) { @@ -150,9 +143,9 @@ struct CPUSuffix { }; static constexpr CPUSuffix Suffixes[] = { -{{"hexagonv4"}, {"4"}}, {{"hexagonv5"}, {"5"}}, -{{"hexagonv55"}, {"55"}}, {{"hexagonv60"}, {"60"}}, -{{"hexagonv62"}, {"62"}}, {{"hexagonv65"}, {"65"}}, +{{"hexagonv5"}, {"5"}}, {{"hexagonv55"}, {"55"}}, +{{"hexagonv60"}, {"60"}}, {{"hexagonv62"}, {"62"}}, +{{"hexagonv65"}, {"65"}}, }; const char *HexagonTargetInfo::getHexagonCPUSuffix(StringRef Name) { Modified: cfe/trunk/test/Driver/hexagon-toolchain-elf.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-toolchain-elf.c?rev=344786&r1=344785&r2=344786&view=diff == --- cfe/trunk/test/Driver/hexagon-toolchain-elf.c (original) +++ cfe/trunk/test/Driver/hexagon-toolchain-elf.c Fri Oct 19 08:36:45 2018 @@ -59,14 +59,6 @@ // - // RUN: %clang -### -target hexagon-unknown-elf \ // RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \ -// RUN: -mcpu=hexagonv4 \ -// RUN: %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK020 %s -// CHECK020: "-cc1" {{.*}} "-target-cpu" "hexagonv4" -// CHECK020: {{hexagon-link|ld}}{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v4/crt0 - -// RUN: %clang -### -target hexagon-unknown-elf \ -// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \ // RUN: -mcpu=hexagonv5 \ // RUN: %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK021 %s Modified: cfe/trunk/test/Misc/target-invalid-cpu-note.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/target-invalid-cpu-note.c?rev=344786&r1=344785&r2=344786&view=diff == --- cfe/trunk/test/Misc/target-invalid-cpu-n
[PATCH] D53404: [clangd] Set workspace root when initializing ClangdServer, disallow mutation.
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE344787: [clangd] Set workspace root when initializing ClangdServer, disallow mutation. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D53404?vs=170098&id=170204#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53404 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h unittests/clangd/FindSymbolsTests.cpp Index: unittests/clangd/FindSymbolsTests.cpp === --- unittests/clangd/FindSymbolsTests.cpp +++ unittests/clangd/FindSymbolsTests.cpp @@ -42,6 +42,7 @@ ClangdServer::Options optsForTests() { auto ServerOpts = ClangdServer::optsForTest(); + ServerOpts.WorkspaceRoot = testRoot(); ServerOpts.BuildDynamicSymbolIndex = true; ServerOpts.URISchemes = {"unittest", "file"}; return ServerOpts; @@ -53,7 +54,6 @@ : Server(CDB, FSProvider, DiagConsumer, optsForTests()) { // Make sure the test root directory is created. FSProvider.Files[testPath("unused")] = ""; -Server.setRootPath(testRoot()); } protected: Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -86,6 +86,11 @@ /// If set, use this index to augment code completion results. SymbolIndex *StaticIndex = nullptr; +/// Clangd's workspace root. Relevant for "workspace" operations not bound +/// to a particular file. +/// FIXME: If not set, should use the current working directory. +llvm::Optional WorkspaceRoot; + /// The resource directory is used to find internal headers, overriding /// defaults and -resource-dir compiler flag). /// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to @@ -116,9 +121,6 @@ const FileSystemProvider &FSProvider, DiagnosticsConsumer &DiagConsumer, const Options &Opts); - /// Set the root path of the workspace. - void setRootPath(PathRef RootPath); - /// Add a \p File to the list of tracked C++ files or update the contents if /// \p File is already tracked. Also schedules parsing of the AST for it on a /// separate thread. When the parsing is complete, DiagConsumer passed in @@ -255,8 +257,7 @@ CachedCompletionFuzzyFindRequestByFile; mutable std::mutex CachedCompletionFuzzyFindRequestMutex; - // If set, this represents the workspace path. - llvm::Optional RootPath; + llvm::Optional WorkspaceRoot; std::shared_ptr PCHs; /// Used to serialize diagnostic callbacks. /// FIXME(ibiryukov): get rid of an extra map and put all version counters Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -109,6 +109,7 @@ ? new FileIndex(Opts.URISchemes, Opts.HeavyweightDynamicSymbolIndex) : nullptr), + WorkspaceRoot(Opts.WorkspaceRoot), PCHs(std::make_shared()), // Pass a callback into `WorkScheduler` to extract symbols from a newly // parsed file and rebuild the file index synchronously each time an AST @@ -131,18 +132,6 @@ Index = nullptr; } -void ClangdServer::setRootPath(PathRef RootPath) { - auto FS = FSProvider.getFileSystem(); - auto Status = FS->status(RootPath); - if (!Status) -elog("Failed to get status for RootPath {0}: {1}", RootPath, - Status.getError().message()); - else if (Status->isDirectory()) -this->RootPath = RootPath; - else -elog("The provided RootPath {0} is not a directory.", RootPath); -} - void ClangdServer::addDocument(PathRef File, StringRef Contents, WantDiagnostics WantDiags) { DocVersion Version = ++InternalVersion[File]; @@ -495,7 +484,7 @@ void ClangdServer::workspaceSymbols( StringRef Query, int Limit, Callback> CB) { CB(clangd::getWorkspaceSymbols(Query, Limit, Index, - RootPath ? *RootPath : "")); + WorkspaceRoot.getValueOr(""))); } void ClangdServer::documentSymbols( Index: clangd/ClangdLSPServer.cpp === --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -261,6 +261,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, Callback Reply) { + if (Params.rootUri && *Params.rootUri) +ClangdServerOpts.WorkspaceRoot = Params.rootUri->file(); + else if (Params.rootPath && !Params.rootPath->empty()) +ClangdServerOpts.WorkspaceRoot = *Params.rootPath; if (Server) return Reply(make_error("server already initialized", ErrorCode::InvalidRequest)); @@ -277,11 +281,6
[clang-tools-extra] r344787 - [clangd] Set workspace root when initializing ClangdServer, disallow mutation.
Author: sammccall Date: Fri Oct 19 08:42:23 2018 New Revision: 344787 URL: http://llvm.org/viewvc/llvm-project?rev=344787&view=rev Log: [clangd] Set workspace root when initializing ClangdServer, disallow mutation. Summary: Rename instance variable to WorkspaceRoot to match what we call it internally. Add fixme to set it automatically. Don't do it yet, clients have assumptions that the constructor won't access the FS. Don't second-guess the provided root. Reviewers: ioeric Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits Differential Revision: https://reviews.llvm.org/D53404 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344787&r1=344786&r2=344787&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Oct 19 08:42:23 2018 @@ -261,6 +261,10 @@ void ClangdLSPServer::reply(llvm::json:: void ClangdLSPServer::onInitialize(const InitializeParams &Params, Callback Reply) { + if (Params.rootUri && *Params.rootUri) +ClangdServerOpts.WorkspaceRoot = Params.rootUri->file(); + else if (Params.rootPath && !Params.rootPath->empty()) +ClangdServerOpts.WorkspaceRoot = *Params.rootPath; if (Server) return Reply(make_error("server already initialized", ErrorCode::InvalidRequest)); @@ -277,11 +281,6 @@ void ClangdLSPServer::onInitialize(const applyConfiguration(Opts.ParamsChange); } - if (Params.rootUri && *Params.rootUri) -Server->setRootPath(Params.rootUri->file()); - else if (Params.rootPath && !Params.rootPath->empty()) -Server->setRootPath(*Params.rootPath); - CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets; DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes; DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory; Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=344787&r1=344786&r2=344787&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Oct 19 08:42:23 2018 @@ -109,6 +109,7 @@ ClangdServer::ClangdServer(const GlobalC ? new FileIndex(Opts.URISchemes, Opts.HeavyweightDynamicSymbolIndex) : nullptr), + WorkspaceRoot(Opts.WorkspaceRoot), PCHs(std::make_shared()), // Pass a callback into `WorkScheduler` to extract symbols from a newly // parsed file and rebuild the file index synchronously each time an AST @@ -131,18 +132,6 @@ ClangdServer::ClangdServer(const GlobalC Index = nullptr; } -void ClangdServer::setRootPath(PathRef RootPath) { - auto FS = FSProvider.getFileSystem(); - auto Status = FS->status(RootPath); - if (!Status) -elog("Failed to get status for RootPath {0}: {1}", RootPath, - Status.getError().message()); - else if (Status->isDirectory()) -this->RootPath = RootPath; - else -elog("The provided RootPath {0} is not a directory.", RootPath); -} - void ClangdServer::addDocument(PathRef File, StringRef Contents, WantDiagnostics WantDiags) { DocVersion Version = ++InternalVersion[File]; @@ -495,7 +484,7 @@ void ClangdServer::onFileEvent(const Did void ClangdServer::workspaceSymbols( StringRef Query, int Limit, Callback> CB) { CB(clangd::getWorkspaceSymbols(Query, Limit, Index, - RootPath ? *RootPath : "")); + WorkspaceRoot.getValueOr(""))); } void ClangdServer::documentSymbols( Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=344787&r1=344786&r2=344787&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Oct 19 08:42:23 2018 @@ -86,6 +86,11 @@ public: /// If set, use this index to augment code completion results. SymbolIndex *StaticIndex = nullptr; +/// Clangd's workspace root. Relevant for "workspace" operations not bound +/// to a particular file. +/// FIXME: If not set, should use the current working directory. +llvm::Optional Wo
[PATCH] D53434: Java annotation declaration being handled correctly
thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks for the fix! Do you happen to know what had regressed this? Repository: rC Clang https://reviews.llvm.org/D53434 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change
twoh added a comment. @joerg Sorry but I'm not sure if I understand your question. This doesn't pretend to honor source code order, but makes linker to place "hot" functions under .text.hot section (There's no guarantee of ordering between functions inside .hot.text section) while "cold" functions under .text.unlikely section. This is purely for performance. Repository: rC Clang https://reviews.llvm.org/D34796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change
twoh updated this revision to Diff 170205. twoh added a comment. Remove conflict line. Repository: rC Clang https://reviews.llvm.org/D34796 Files: docs/ClangCommandLineReference.rst include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/Inputs/freorder-functions.prof test/CodeGen/freorder-functions.c test/Driver/function-sections.c Index: test/Driver/function-sections.c === --- test/Driver/function-sections.c +++ test/Driver/function-sections.c @@ -6,6 +6,8 @@ // CHECK-NODS-NOT: -fdata-sections // CHECK-US-NOT: -fno-unique-section-names // CHECK-NOUS: -fno-unique-section-names +// CHECK-RF-NOT: -fno-reorder-functions +// CHECK-NORF: -fno-reorder-functions // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -72,3 +74,13 @@ // RUN: -target i386-unknown-linux \ // RUN: -fno-unique-section-names \ // RUN: | FileCheck --check-prefix=CHECK-NOUS %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\ +// RUN: -target i386-unknown-linux \ +// RUN: -freorder-functions \ +// RUN: | FileCheck --check-prefix=CHECK-RF %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\ +// RUN: -target i386-unknown-linux \ +// RUN: -fno-reorder-functions \ +// RUN: | FileCheck --check-prefix=CHECK-NORF %s Index: test/CodeGen/freorder-functions.c === --- /dev/null +++ test/CodeGen/freorder-functions.c @@ -0,0 +1,22 @@ +// REQUIRES: x86-registered-target + +// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -o - < %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -fno-reorder-functions -o - < %s | FileCheck --check-prefix=CHECK-NOPREFIX %s + +// opt tool option precedes driver option. +// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -fno-reorder-functions -mllvm -profile-guided-section-prefix=true -o - < %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -freorder-functions -mllvm -profile-guided-section-prefix=false -o - < %s | FileCheck --check-prefix=CHECK-NOPREFIX %s + +void hot_func() { + return; +} + +void cold_func() { + hot_func(); + return; +} + +// CHECK: .section .text.hot.hot_func,"ax",@progbits +// CHECK: .section .text.unlikely.cold_func,"ax",@progbits +// CHECK-NOPREFIX: .section .text.hot_func,"ax",@progbits +// CHECK-NOPREFIX: .section .text.cold_func,"ax",@progbits Index: test/CodeGen/Inputs/freorder-functions.prof === --- /dev/null +++ test/CodeGen/Inputs/freorder-functions.prof @@ -0,0 +1,5 @@ +hot_func:1000:0 + 1: 0 +cold_func:0:0 + 1: 1 + 2: 1 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -756,6 +756,8 @@ Args.hasFlag(OPT_fstack_size_section, OPT_fno_stack_size_section, false); Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names, OPT_fno_unique_section_names, true); + Opts.ReorderFunctions = + Args.hasFlag(OPT_freorder_functions, OPT_fno_reorder_functions, true); Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions); Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -3891,6 +3891,10 @@ options::OPT_fno_unique_section_names, true)) CmdArgs.push_back("-fno-unique-section-names"); + if (!Args.hasFlag(options::OPT_freorder_functions, +options::OPT_fno_reorder_functions, true)) +CmdArgs.push_back("-fno-reorder-functions"); + if (auto *A = Args.getLastArg( options::OPT_finstrument_functions, options::OPT_finstrument_functions_after_inlining, @@ -3929,6 +3933,82 @@ Args.AddLastArg(CmdArgs, options::OPT_working_directory); + bool ARCMTEnabled = false; + if (!Args.hasArg(options::OPT_fno_objc_arc, options::OPT_fobjc_arc)) { +if (const Arg *A = Args.getLastArg(options::OPT_ccc_arcmt_check, + options::OPT_ccc_arcmt_modify, + options::OPT_ccc_arcmt_migrate)) { + ARCMTEnabled = true; + switch (A->getOption().getID()) { + default: +llvm_unreachable("missed a case"); +
[PATCH] D53439: [clangd] Remove caching of compilation database commands.
sammccall created this revision. sammccall added reviewers: ioeric, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. The CDB implementations used in open-source code are fast, and our private slow CDB will soon do the relevant caching itself. Simplifying the GlobalCDB layer in clangd is important to get auto-index implemented at the right layer. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53439 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h Index: clangd/GlobalCompilationDatabase.h === --- clangd/GlobalCompilationDatabase.h +++ clangd/GlobalCompilationDatabase.h @@ -86,33 +86,6 @@ llvm::Optional CompileCommandsDir; }; -/// A wrapper around GlobalCompilationDatabase that caches the compile commands. -/// Note that only results of getCompileCommand are cached. -class CachingCompilationDb : public GlobalCompilationDatabase { -public: - explicit CachingCompilationDb(const GlobalCompilationDatabase &InnerCDB); - - /// Gets compile command for \p File from cache or CDB if it's not in the - /// cache. - llvm::Optional - getCompileCommand(PathRef File) const override; - - /// Forwards to the inner CDB. Results of this function are not cached. - tooling::CompileCommand getFallbackCommand(PathRef File) const override; - - /// Removes an entry for \p File if it's present in the cache. - void invalidate(PathRef File); - - /// Removes all cached compile commands. - void clear(); - -private: - const GlobalCompilationDatabase &InnerCDB; - mutable std::mutex Mut; - mutable llvm::StringMap> - Cached; /* GUARDED_BY(Mut) */ -}; - /// Gets compile args from an in-memory mapping based on a filepath. Typically /// used by clients who provide the compile commands themselves. class InMemoryCompilationDb : public GlobalCompilationDatabase { Index: clangd/GlobalCompilationDatabase.cpp === --- clangd/GlobalCompilationDatabase.cpp +++ clangd/GlobalCompilationDatabase.cpp @@ -117,39 +117,6 @@ return nullptr; } -CachingCompilationDb::CachingCompilationDb( -const GlobalCompilationDatabase &InnerCDB) -: InnerCDB(InnerCDB) {} - -llvm::Optional -CachingCompilationDb::getCompileCommand(PathRef File) const { - std::unique_lock Lock(Mut); - auto It = Cached.find(File); - if (It != Cached.end()) -return It->second; - - Lock.unlock(); - llvm::Optional Command = - InnerCDB.getCompileCommand(File); - Lock.lock(); - return Cached.try_emplace(File, std::move(Command)).first->getValue(); -} - -tooling::CompileCommand -CachingCompilationDb::getFallbackCommand(PathRef File) const { - return InnerCDB.getFallbackCommand(File); -} - -void CachingCompilationDb::invalidate(PathRef File) { - std::unique_lock Lock(Mut); - Cached.erase(File); -} - -void CachingCompilationDb::clear() { - std::unique_lock Lock(Mut); - Cached.clear(); -} - llvm::Optional InMemoryCompilationDb::getCompileCommand(PathRef File) const { std::lock_guard Lock(Mutex); Index: clangd/ClangdLSPServer.h === --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -135,21 +135,17 @@ /// Returns a CDB that should be used to get compile commands for the /// current instance of ClangdLSPServer. -GlobalCompilationDatabase &getCDB(); +GlobalCompilationDatabase &getCDB() { return *CDB; } private: CompilationDB(std::unique_ptr CDB, - std::unique_ptr CachingCDB, bool IsDirectoryBased) -: CDB(std::move(CDB)), CachingCDB(std::move(CachingCDB)), - IsDirectoryBased(IsDirectoryBased) {} +: CDB(std::move(CDB)), IsDirectoryBased(IsDirectoryBased) {} // if IsDirectoryBased is true, an instance of InMemoryCDB. // If IsDirectoryBased is false, an instance of DirectoryBasedCDB. // unique_ptr CDB; std::unique_ptr CDB; -// Non-null only for directory-based CDB -std::unique_ptr CachingCDB; bool IsDirectoryBased; }; Index: clangd/ClangdLSPServer.cpp === --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -787,25 +787,22 @@ } ClangdLSPServer::CompilationDB ClangdLSPServer::CompilationDB::makeInMemory() { - return CompilationDB(llvm::make_unique(), nullptr, + return CompilationDB(llvm::make_unique(), /*IsDirectoryBased=*/false); } ClangdLSPServer::CompilationDB ClangdLSPServer::CompilationDB::makeDirectoryBased( llvm::Optional CompileCommandsDir) { auto CDB = llvm::make_unique( std::move(CompileCommandsDir)); - auto CachingCDB = llvm::make_unique(*CDB); - return CompilationDB(std::move(CDB), std::move(CachingCDB), + return CompilationDB(std::
[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.
sammccall added a comment. Nice savings! My initial thought is that it seems weird to only do it for this one string - the readers look different, the serialization code has different logic etc for the different flavors of strings. The struct certainly looks nicer to my eyes. This may be the right tradeoff, but I'd like to play with/think about other options for API here. One is to define a class like `NullTerminatedStringRef` (with a better name) that wraps a char* only, but otherwise behaves as much like stringref as we can manage. Happy for you to try some of these out or to try them myself. I think this is worth a bit of thought before committing because as shown in this patch, these APIs have quite a lot of uses! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344789 - Java annotation declaration being handled correctly
Author: hans Date: Fri Oct 19 09:19:52 2018 New Revision: 344789 URL: http://llvm.org/viewvc/llvm-project?rev=344789&view=rev Log: Java annotation declaration being handled correctly Previously, Java annotation declarations (@interface AnnotationName) were being handled as ObjC interfaces. This caused the brace formatting to mess up, so that when you had a class with an interface defined in it, it would indent the final brace of the class. It used to format this class like so: class A { @interface B {} } But will now just skip the @interface and format it like so: class A { @interface B {} } Patch by Sam Maier! Differential Revision: https://reviews.llvm.org/D53434 Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTestJava.cpp Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=344789&r1=344788&r2=344789&view=diff == --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original) +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Fri Oct 19 09:19:52 2018 @@ -1130,6 +1130,10 @@ void UnwrappedLineParser::parseStructura nextToken(); parseBracedList(); break; + } else if (Style.Language == FormatStyle::LK_Java && + FormatTok->is(Keywords.kw_interface)) { +nextToken(); +break; } switch (FormatTok->Tok.getObjCKeywordID()) { case tok::objc_public: Modified: cfe/trunk/unittests/Format/FormatTestJava.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJava.cpp?rev=344789&r1=344788&r2=344789&view=diff == --- cfe/trunk/unittests/Format/FormatTestJava.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestJava.cpp Fri Oct 19 09:19:52 2018 @@ -155,6 +155,15 @@ TEST_F(FormatTestJava, ClassDeclarations " void doStuff(int theStuff);\n" " void doMoreStuff(int moreStuff);\n" "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {\n" + "int stuff;\n" + "void doMoreStuff(int moreStuff);\n" + " }\n" + "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {}\n" + "}"); } TEST_F(FormatTestJava, AnonymousClasses) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53434: Java annotation declaration being handled correctly
This revision was automatically updated to reflect the committed changes. Closed by commit rL344789: Java annotation declaration being handled correctly (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53434?vs=170197&id=170210#toc Repository: rL LLVM https://reviews.llvm.org/D53434 Files: cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTestJava.cpp Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp === --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp @@ -1130,6 +1130,10 @@ nextToken(); parseBracedList(); break; + } else if (Style.Language == FormatStyle::LK_Java && + FormatTok->is(Keywords.kw_interface)) { +nextToken(); +break; } switch (FormatTok->Tok.getObjCKeywordID()) { case tok::objc_public: Index: cfe/trunk/unittests/Format/FormatTestJava.cpp === --- cfe/trunk/unittests/Format/FormatTestJava.cpp +++ cfe/trunk/unittests/Format/FormatTestJava.cpp @@ -155,6 +155,15 @@ " void doStuff(int theStuff);\n" " void doMoreStuff(int moreStuff);\n" "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {\n" + "int stuff;\n" + "void doMoreStuff(int moreStuff);\n" + " }\n" + "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {}\n" + "}"); } TEST_F(FormatTestJava, AnonymousClasses) { Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp === --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp @@ -1130,6 +1130,10 @@ nextToken(); parseBracedList(); break; + } else if (Style.Language == FormatStyle::LK_Java && + FormatTok->is(Keywords.kw_interface)) { +nextToken(); +break; } switch (FormatTok->Tok.getObjCKeywordID()) { case tok::objc_public: Index: cfe/trunk/unittests/Format/FormatTestJava.cpp === --- cfe/trunk/unittests/Format/FormatTestJava.cpp +++ cfe/trunk/unittests/Format/FormatTestJava.cpp @@ -155,6 +155,15 @@ " void doStuff(int theStuff);\n" " void doMoreStuff(int moreStuff);\n" "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {\n" + "int stuff;\n" + "void doMoreStuff(int moreStuff);\n" + " }\n" + "}"); + verifyFormat("class A {\n" + " public @interface SomeInterface {}\n" + "}"); } TEST_F(FormatTestJava, AnonymousClasses) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.
ilya-biryukov added a comment. A few more comments, mostly marking places of unintentional changes that we need to revert. Hope it's not going past the point where the number of comments are not useful. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8616 "use of %select{type|declaration}0 %1 requires %2 extension to be enabled">; -def warn_opencl_generic_address_space_arg : Warning< - "passing non-generic address space pointer to %0" This looks unintentional. Should we revert? Comment at: lib/Sema/SemaChecking.cpp:852 - if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) { -S.Diag(Call->getArg(0)->getBeginLoc(), aaron.ballman wrote: > kadircet wrote: > > aaron.ballman wrote: > > > I'm not certain I understand why this code has been removed? > > It shouldn't have been, tried rebasing but it didn't go away. I think it > > was deleted at some point by a different change and somehow ended up > > showing in here as well. (Tried to revert, got an error stating > > warn_opencl_generic_address_space_arg doesn't exist) > That's truly strange. I bring it up because these sort of changes make it > harder for reviewers to test changes locally by applying the patch themselves. +1, we definitely need to revert this, happy to help figure out why this happened. Comment at: lib/Sema/SemaTemplateDeduction.cpp:4348 TLB.reserve(TL.getFullDataSize()); - return TransformType(TLB, TL); + QualType Result = TransformType(TLB, TL); + return TLB.getTypeSourceInfo(getSema().Context, Result); Maybe remove the intermediate variable? Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:354 bool VisitTypeLoc(TypeLoc Loc) { +if(Loc.getTypeLocClass() == TypeLoc::Auto) + return true; Why do we need this change? Are we fixing some particular regressions? A comment would be useful. Comment at: test/SemaOpenCL/to_addr_builtin.cl:2 // RUN: %clang_cc1 -verify -fsyntax-only %s -// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s Unrelated change. Comment at: test/SemaOpenCL/to_addr_builtin.cl:48 // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}} - // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}} #endif Unrelated change Comment at: test/SemaOpenCL/to_addr_builtin.cl:122 // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}} - // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}} #endif Another unrelated change Repository: rC Clang https://reviews.llvm.org/D52301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53441: [ms] Prevent explicit constructor name lookup if scope is missing
lantictac created this revision. lantictac added a reviewer: rsmith. lantictac added a project: clang. MicrosoftExt allows explicit constructor calls. Prevent lookup of constructor name unless the name has explicit scope. This avoids a compile-time crash due to confusing a member access for a constructor name. Test case included. All tests pass. Repository: rC Clang https://reviews.llvm.org/D53441 Files: lib/Parse/ParseExpr.cpp test/SemaCXX/MicrosoftCompatibility.cpp Index: test/SemaCXX/MicrosoftCompatibility.cpp === --- test/SemaCXX/MicrosoftCompatibility.cpp +++ test/SemaCXX/MicrosoftCompatibility.cpp @@ -302,3 +302,23 @@ void *a2 = &function_prototype; // expected-warning {{implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension}} void *a3 = function_ptr;// expected-warning {{implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension}} } + +namespace member_lookup { + +template +struct ConfuseLookup { + T* m_val; + struct m_val { +static size_t ms_test; + }; +}; + +// Microsoft mode allows explicit constructor calls +// This could confuse name lookup in cases such as this +template +size_t ConfuseLookup::m_val::ms_test + = size_t(&(char&)(reinterpret_cast*>(0)->m_val)); + +void instantiate() { ConfuseLookup::m_val::ms_test = 1; } +} + Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -1809,7 +1809,8 @@ /*EnteringContext=*/false, /*AllowDestructorName=*/true, /*AllowConstructorName=*/ - getLangOpts().MicrosoftExt, +getLangOpts().MicrosoftExt && +SS.isNotEmpty(), /*AllowDeductionGuide=*/false, ObjectType, &TemplateKWLoc, Name)) { (void)Actions.CorrectDelayedTyposInExpr(LHS); Index: test/SemaCXX/MicrosoftCompatibility.cpp === --- test/SemaCXX/MicrosoftCompatibility.cpp +++ test/SemaCXX/MicrosoftCompatibility.cpp @@ -302,3 +302,23 @@ void *a2 = &function_prototype; // expected-warning {{implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension}} void *a3 = function_ptr;// expected-warning {{implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension}} } + +namespace member_lookup { + +template +struct ConfuseLookup { + T* m_val; + struct m_val { +static size_t ms_test; + }; +}; + +// Microsoft mode allows explicit constructor calls +// This could confuse name lookup in cases such as this +template +size_t ConfuseLookup::m_val::ms_test + = size_t(&(char&)(reinterpret_cast*>(0)->m_val)); + +void instantiate() { ConfuseLookup::m_val::ms_test = 1; } +} + Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -1809,7 +1809,8 @@ /*EnteringContext=*/false, /*AllowDestructorName=*/true, /*AllowConstructorName=*/ - getLangOpts().MicrosoftExt, +getLangOpts().MicrosoftExt && +SS.isNotEmpty(), /*AllowDeductionGuide=*/false, ObjectType, &TemplateKWLoc, Name)) { (void)Actions.CorrectDelayedTyposInExpr(LHS); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53369: [CodeComplete] Fix accessibility of protected members when accessing members implicitly.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM, thanks! Would be super-nice if didn't have to rewrite this in code completion Repository: rC Clang https://reviews.llvm.org/D53369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.
kadircet updated this revision to Diff 170215. kadircet added a comment. - Keep only relavant changes and rebase Repository: rC Clang https://reviews.llvm.org/D52301 Files: include/clang/AST/PrettyPrinter.h include/clang/Sema/Sema.h lib/AST/TypePrinter.cpp lib/Frontend/ASTConsumers.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaLambda.cpp lib/Sema/SemaStmt.cpp lib/Sema/SemaTemplate.cpp lib/Sema/SemaTemplateDeduction.cpp Index: lib/Sema/SemaTemplateDeduction.cpp === --- lib/Sema/SemaTemplateDeduction.cpp +++ lib/Sema/SemaTemplateDeduction.cpp @@ -4340,19 +4340,20 @@ return E; } -QualType Apply(TypeLoc TL) { +TypeSourceInfo* Apply(TypeLoc TL) { // Create some scratch storage for the transformed type locations. // FIXME: We're just going to throw this information away. Don't build it. TypeLocBuilder TLB; TLB.reserve(TL.getFullDataSize()); - return TransformType(TLB, TL); + QualType Result = TransformType(TLB, TL); + return TLB.getTypeSourceInfo(getSema().Context, Result); } }; } // namespace Sema::DeduceAutoResult -Sema::DeduceAutoType(TypeSourceInfo *Type, Expr *&Init, QualType &Result, +Sema::DeduceAutoType(TypeSourceInfo *Type, Expr *&Init, TypeSourceInfo *&Result, Optional DependentDeductionDepth) { return DeduceAutoType(Type->getTypeLoc(), Init, Result, DependentDeductionDepth); @@ -4398,7 +4399,7 @@ ///'auto' template parameters. The value specified is the template ///parameter depth at which we should perform 'auto' deduction. Sema::DeduceAutoResult -Sema::DeduceAutoType(TypeLoc Type, Expr *&Init, QualType &Result, +Sema::DeduceAutoType(TypeLoc Type, Expr *&Init, TypeSourceInfo *&Result, Optional DependentDeductionDepth) { if (Init->getType()->isNonOverloadPlaceholderType()) { ExprResult NonPlaceholder = CheckPlaceholderExpr(Init); @@ -4410,7 +4411,8 @@ if (!DependentDeductionDepth && (Type.getType()->isDependentType() || Init->isTypeDependent())) { Result = SubstituteDeducedTypeTransform(*this, QualType()).Apply(Type); -assert(!Result.isNull() && "substituting DependentTy can't fail"); +assert(Result && !Result->getType().isNull() && + "substituting DependentTy can't fail"); return DAR_Succeeded; } @@ -4433,7 +4435,7 @@ // FIXME: Support a non-canonical deduced type for 'auto'. Deduced = Context.getCanonicalType(Deduced); Result = SubstituteDeducedTypeTransform(*this, Deduced).Apply(Type); - if (Result.isNull()) + if (!Result || Result->getType().isNull()) return DAR_FailedAlreadyDiagnosed; return DAR_Succeeded; } else if (!getLangOpts().CPlusPlus) { @@ -4456,10 +4458,10 @@ FixedSizeTemplateParameterListStorage<1, false> TemplateParamsSt( Loc, Loc, TemplParamPtr, Loc, nullptr); - QualType FuncParam = - SubstituteDeducedTypeTransform(*this, TemplArg, /*UseTypeSugar*/false) + TypeSourceInfo *FuncParamTSI = + SubstituteDeducedTypeTransform(*this, TemplArg, /*UseTypeSugar*/ false) .Apply(Type); - assert(!FuncParam.isNull() && + assert(!FuncParamTSI->getType().isNull() && "substituting template parameter for 'auto' failed"); // Deduce type of TemplParam in Func(Init) @@ -4474,7 +4476,8 @@ ArrayRef Ranges) -> DeduceAutoResult { if (Init->isTypeDependent()) { Result = SubstituteDeducedTypeTransform(*this, QualType()).Apply(Type); - assert(!Result.isNull() && "substituting DependentTy can't fail"); + assert(Result && !Result->getType().isNull() && + "substituting DependentTy can't fail"); return DAR_Succeeded; } if (diagnoseAutoDeductionFailure(*this, TDK, Info, Ranges)) @@ -4514,8 +4517,9 @@ } if (auto TDK = DeduceTemplateArgumentsFromCallArgument( -*this, TemplateParamsSt.get(), 0, FuncParam, Init, Info, Deduced, -OriginalCallArgs, /*Decomposed*/ false, /*ArgIdx*/ 0, /*TDF*/ 0)) +*this, TemplateParamsSt.get(), 0, FuncParamTSI->getType(), Init, +Info, Deduced, OriginalCallArgs, /*Decomposed*/ false, /*ArgIdx*/ 0, +/*TDF*/ 0)) return DeductionFailed(TDK, {}); } @@ -4532,18 +4536,18 @@ } Result = SubstituteDeducedTypeTransform(*this, DeducedType).Apply(Type); - if (Result.isNull()) + if (!Result || Result->getType().isNull()) return DAR_FailedAlreadyDiagnosed; // Check that the deduced argument type is compatible with the original // argument type per C++ [temp.deduct.call]p4. - QualType DeducedA = InitList ? Deduced[0].getAsType() : Result; + QualType DeducedA = InitList ? Deduced[0].getAsType() : Result->getType(); for (const OriginalCallArg &OriginalArg : OriginalCallArgs) { assert((bool
[PATCH] D52654: [OpenCL][NFC] Unify ZeroToOCL* cast types
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Thanks! https://reviews.llvm.org/D52654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.
gtbercea created this revision. gtbercea added reviewers: ABataev, caomhin. Herald added subscribers: cfe-commits, guansong, jholewinski. This patch enables the choosing of the default schedule for parallel for loops even in non-SPMD cases. Repository: rC Clang https://reviews.llvm.org/D53443 Files: lib/CodeGen/CGOpenMPRuntime.h lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp lib/CodeGen/CGOpenMPRuntimeNVPTX.h lib/CodeGen/CGStmtOpenMP.cpp Index: lib/CodeGen/CGStmtOpenMP.cpp === --- lib/CodeGen/CGStmtOpenMP.cpp +++ lib/CodeGen/CGStmtOpenMP.cpp @@ -2313,7 +2313,7 @@ } else { // Default behaviour for schedule clause. CGM.getOpenMPRuntime().getDefaultScheduleAndChunk( -*this, S, ScheduleKind.Schedule, Chunk); +*this, S, ScheduleKind, Chunk); } const unsigned IVSize = getContext().getTypeSize(IVExpr->getType()); const bool IVSigned = IVExpr->getType()->hasSignedIntegerRepresentation(); Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.h +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h @@ -347,7 +347,7 @@ /// Choose a default value for the schedule clause. void getDefaultScheduleAndChunk(CodeGenFunction &CGF, - const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, + const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind, llvm::Value *&Chunk) const override; private: Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp @@ -4238,16 +4238,17 @@ Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF), CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0), S.getIterationVariable()->getType(), S.getBeginLoc()); +return; } + CGOpenMPRuntime::getDefaultDistScheduleAndChunk( + CGF, S, ScheduleKind, Chunk); } void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk( CodeGenFunction &CGF, const OMPLoopDirective &S, -OpenMPScheduleClauseKind &ScheduleKind, +OpenMPScheduleTy &ScheduleKind, llvm::Value *&Chunk) const { - if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) { -ScheduleKind = OMPC_SCHEDULE_static; -Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( -S.getIterationVariable()->getType()), 1); - } + ScheduleKind.Schedule = OMPC_SCHEDULE_static; + Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( + S.getIterationVariable()->getType()), 1); } Index: lib/CodeGen/CGOpenMPRuntime.h === --- lib/CodeGen/CGOpenMPRuntime.h +++ lib/CodeGen/CGOpenMPRuntime.h @@ -1505,7 +1505,7 @@ /// Choose default schedule type and chunk value for the /// schedule clause. virtual void getDefaultScheduleAndChunk(CodeGenFunction &CGF, - const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, + const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind, llvm::Value *&Chunk) const {} /// Emits call of the outlined function with the provided arguments, Index: lib/CodeGen/CGStmtOpenMP.cpp === --- lib/CodeGen/CGStmtOpenMP.cpp +++ lib/CodeGen/CGStmtOpenMP.cpp @@ -2313,7 +2313,7 @@ } else { // Default behaviour for schedule clause. CGM.getOpenMPRuntime().getDefaultScheduleAndChunk( -*this, S, ScheduleKind.Schedule, Chunk); +*this, S, ScheduleKind, Chunk); } const unsigned IVSize = getContext().getTypeSize(IVExpr->getType()); const bool IVSigned = IVExpr->getType()->hasSignedIntegerRepresentation(); Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.h +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h @@ -347,7 +347,7 @@ /// Choose a default value for the schedule clause. void getDefaultScheduleAndChunk(CodeGenFunction &CGF, - const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, + const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind, llvm::Value *&Chunk) const override; private: Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp @@ -4238,16 +4238,17 @@ Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF), CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0), S.getIterationVariable()->getType(), S.getBeginLoc()); +return; } + CGOpenMPRuntime::getDefaultDistScheduleAndChunk( + CGF, S, ScheduleKind, Chunk); } void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk( CodeGenFunction &CGF, con
[PATCH] D53200: [OpenCL] Fix serialization of OpenCLExtensionDecls
Anastasia accepted this revision. Anastasia added inline comments. This revision is now accepted and ready to land. Comment at: test/Headers/opencl-pragma-extension-begin.cl:1 +// RUN: rm -rf %t +// RUN: mkdir -p %t I think the tests in this folder are for standard includes only but you are testing custom include file here. Could this be integrated into test/SemaOpenCL/extension-begin.cl? Or if else you could just move to that folder (it might be better to append module to the name in that case). Comment at: test/Headers/opencl-pragma-extension-begin.cl:11 + +void __kernel test(__global int *data) { + *data = 10; May be it makes sense to test the added extension? https://reviews.llvm.org/D53200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344795 - [Driver] Reland: Default Android toolchains to libc++.
Author: danalbert Date: Fri Oct 19 11:06:02 2018 New Revision: 344795 URL: http://llvm.org/viewvc/llvm-project?rev=344795&view=rev Log: [Driver] Reland: Default Android toolchains to libc++. The sanitizer builder that was broken by this should now be fixed. Original review was https://reviews.llvm.org/D53109 Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/test/Driver/android-ndk-standalone.cpp Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=344795&r1=344794&r2=344795&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Fri Oct 19 11:06:02 2018 @@ -443,6 +443,12 @@ Linux::Linux(const Driver &D, const llvm addPathIfExists(D, SysRoot + "/usr/lib", Paths); } +ToolChain::CXXStdlibType Linux::GetDefaultCXXStdlibType() const { + if (getTriple().isAndroid()) +return ToolChain::CST_Libcxx; + return ToolChain::CST_Libstdcxx; +} + bool Linux::HasNativeLLVMSupport() const { return true; } Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); } Modified: cfe/trunk/lib/Driver/ToolChains/Linux.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.h?rev=344795&r1=344794&r2=344795&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Linux.h (original) +++ cfe/trunk/lib/Driver/ToolChains/Linux.h Fri Oct 19 11:06:02 2018 @@ -37,6 +37,7 @@ public: llvm::opt::ArgStringList &CC1Args) const override; void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; + CXXStdlibType GetDefaultCXXStdlibType() const override; bool isPIEDefault() const override; bool IsMathErrnoDefault() const override; SanitizerMask getSupportedSanitizers() const override; Modified: cfe/trunk/test/Driver/android-ndk-standalone.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-ndk-standalone.cpp?rev=344795&r1=344794&r2=344795&view=diff == --- cfe/trunk/test/Driver/android-ndk-standalone.cpp (original) +++ cfe/trunk/test/Driver/android-ndk-standalone.cpp Fri Oct 19 11:06:02 2018 @@ -2,21 +2,13 @@ // toolchain. // // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ -// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \ +// RUN: -target arm-linux-androideabi21 \ // RUN: -B%S/Inputs/basic_android_ndk_tree \ // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \ // RUN: | FileCheck %s // CHECK: {{.*}}clang{{.*}}" "-cc1" // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]" -// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9" -// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" -// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a" -// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb" -// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi" -// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" -// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a" -// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb" -// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/backward" +// CHECK: "-internal-isystem" "{{.*}}/include/c++/v1" // CHECK: "-internal-isystem" "{{.*}}/sysroot/usr/local/include" // CHECK: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include" // CHECK: "-internal-externc-isystem" "{{.*}}/sysroot/usr/include/arm-linux-androideabi" @@ -49,21 +41,47 @@ // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi" // // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ -// RUN: -target armv7a-none-linux-androideabi21 -stdlib=libstdc++ \ +// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \ +// RUN: -B%S/Inputs/basic_android_ndk_tree \ +// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \ +// RUN: | FileCheck --check-prefix=CHECK-STDCXX %s +// CHECK-STDCXX: {{.*}}clang{{.*}}" "-cc1" +// CHECK-STDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]" +// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9" +// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" +// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a" +// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb" +// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-
[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.
dblaikie added a comment. @echristo > As far as the standard text here, IMO it was just there in case people didn't > have an objcopy around or don't want to split it. I'm not sure why we would > want the ability. I think others have mentioned - but with distributed build it might be easier to keep it as a single file because your build system already knows how to ship around those files. > That said, if we do I'd rather have it as dwarf5 without split-dwarf as an > option rather than a -gsingle-file-split-dwarf option. Any suggestions on the name? https://reviews.llvm.org/D52296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D51402 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53334: [Frontend] Show diagnostics on prebuilt module configuration mismatch too
dblaikie added a comment. Don't know enough about the code to have an opinion on the fix - but in any case this would need a test case, if possible Repository: rC Clang https://reviews.llvm.org/D53334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.
ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.h:350 void getDefaultScheduleAndChunk(CodeGenFunction &CGF, - const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, + const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind, llvm::Value *&Chunk) const override; Why do you need to change the type of the parameter? Repository: rC Clang https://reviews.llvm.org/D53443 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.
rsmith added a comment. In https://reviews.llvm.org/D52301#1267163, @kadircet wrote: > So in the middle of the patch we were just trying to figure out whether we > are going to break a big assumption that lots of different components relied > on but just didn't had any tests for. Maybe @rsmith would have more context > about the behavior? We used to update the `auto` type in both the semantic type of the variable and in the type-as-written. I changed that in r180808; it was a while back, and I didn't do a good job explaining in the commit message, but I think the only reason was consistency. For the case of a deduced return type in a function, we really want to leave the type in the `TypeSourceInfo` alone, because we need to track the "declared return type" for redeclaration matching: auto f() { return 0; } int f(); // error auto f(); // OK As we need to sometimes track the "declared type" of the entity as distinct from the semantic type, and as the `TypeSourceInfo` is intended to track the type as written, it made sense to consistently not update the `auto` type in the type source information (so the TSI has the declared type, and getType() returns the semantic type). That said... I'm definitely sympathetic to the problem. When using an AST matcher, for instance, we want to simultaneously think about matching certain syntactic source constructs (eg, match a `TypeLoc`) and then ask about the semantic properties of the type (for which you want to know what the deduced type was for an auto type). If we update the `TypeSourceInfo` for variables but not for functions, is that going to deliver the value you're looking for here, or should we be looking for some way to deal with cases where we want the TSI and type to differ and still want to be able to work out correspondences between them? Repository: rC Clang https://reviews.llvm.org/D52301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension
Anastasia added inline comments. Comment at: lib/Headers/opencl-c.h:16197 +#ifdef cl_intel_device_side_avc_motion_estimation +#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable + Anastasia wrote: > Should we be using: > #pragma OPENCL EXTENSION my_ext : begin > then the user can get correct diagnostics that the functions can only be used > once the extension is enabled. Would it be better to use `begin`/`end` instead of `enable`/`disable`? Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:26 + char4 c4, event_t e, struct st ss) { + intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer for mce types + // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with an expression of incompatible type 'int'}} AlexeySachkov wrote: > Anastasia wrote: > > AlexeySachkov wrote: > > > Anastasia wrote: > > > > Would it make sense to add a check for non-zero constant? > > > > > > > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type > > > > from the same type? Any other restrictions on assignment (i.e. w > > > > integer literals) and operations over these types? > > > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type > > > > from the same type? > > > > > > Yes, such assignment is allowed. > > > > > > > Any other restrictions on assignment (i.e. w integer literals) > > > > > > All of these types can only be initialized using call to a special > > > built-ins or using predefined macros like > > > `CLK_AVC_REF_RESULT_INITIALIZE_INTEL`. Any other assignment should lead > > > to an error. > > > > > > I found that I'm able to assign variable of type > > > `intel_sub_group_avc_imc_payload_t` to variable of type > > > `intel_sub_group_avc_mce_payload_t`, so I will update the patch when I > > > implement such diagnostic message. > > > > > > > and operations over these types? > > > Variables of these types can only be used as return values or arguments > > > for built-in functions described in the specification. All other > > > operations are restricted > > W/o the spec change it's really difficult to review properly. So are you > > testing 2 groups of types: > > 1. Init by zero in `bar`? > > 2. Init by builtin function in `foo`? > > > I would like to test: > 1. foo: init by literal or variable, negative tests > 2. far: init by other type or assignment between different types, negative > tests > 3. bar: init by special initializers from spec, positive tests Ok, I see. Considering that the restirctions are not in the spec yet. I feel adding a comment describing initialization/assignment/operation restrictions for different types would be helpful here. Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:16 + +#define CLK_AVC_IME_RESULT_INITIALIZE_INTEL 0x0 +#define CLK_AVC_REF_RESULT_INITIALIZE_INTEL 0x0 Is there any point to have all these defines if they are all mapped to 0? I think using constant literals are easier for testing. https://reviews.llvm.org/D51484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344800 - Add basic test that we perform lifetime extension in the expected
Author: rsmith Date: Fri Oct 19 12:01:31 2018 New Revision: 344800 URL: http://llvm.org/viewvc/llvm-project?rev=344800&view=rev Log: Add basic test that we perform lifetime extension in the expected situations. Added: cfe/trunk/test/CXX/special/class.temporary/p6.cpp Added: cfe/trunk/test/CXX/special/class.temporary/p6.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.temporary/p6.cpp?rev=344800&view=auto == --- cfe/trunk/test/CXX/special/class.temporary/p6.cpp (added) +++ cfe/trunk/test/CXX/special/class.temporary/p6.cpp Fri Oct 19 12:01:31 2018 @@ -0,0 +1,190 @@ +// RUN: %clang_cc1 -std=c++17 %s -emit-llvm -o - | FileCheck %s --implicit-check-not='call{{.*}}dtor' + +void then(); + +struct dtor { + ~dtor(); +}; + +dtor ctor(); + +// [lifetime extension occurs if the object was obtained by] +// -- a temporary materialization conversion +// CHECK-LABEL: ref_binding +void ref_binding() { + // CHECK: call {{.*}}ctor + auto &&x = ctor(); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} + +// -- ( expression ) +// CHECK-LABEL: parens +void parens() { + // CHECK: call {{.*}}ctor + auto &&x = ctor(); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} + +// -- subscripting of an array +// CHECK-LABEL: array_subscript_1 +void array_subscript_1() { + using T = dtor[1]; + // CHECK: call {{.*}}ctor + auto &&x = T{ctor()}[0]; + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} +// CHECK-LABEL: array_subscript_2 +void array_subscript_2() { + using T = dtor[1]; + // CHECK: call {{.*}}ctor + auto &&x = ((dtor*)T{ctor()})[0]; + // CHECK: call {{.*}}dtor + // CHECK: call {{.*}}then + then(); + // CHECK: } +} + +struct with_member { dtor d; ~with_member(); }; +struct with_ref_member { dtor &&d; ~with_ref_member(); }; + +// -- a class member access using the . operator [...] +// CHECK-LABEL: member_access_1 +void member_access_1() { + // CHECK: call {{.*}}ctor + auto &&x = with_member{ctor()}.d; + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}with_member + // CHECK: } +} +// CHECK-LABEL: member_access_2 +void member_access_2() { + // CHECK: call {{.*}}ctor + auto &&x = with_ref_member{ctor()}.d; + // CHECK: call {{.*}}with_ref_member + // CHECK: call {{.*}}dtor + // CHECK: call {{.*}}then + then(); + // CHECK: } +} +// CHECK-LABEL: member_access_3 +void member_access_3() { + // CHECK: call {{.*}}ctor + auto &&x = (&(const with_member&)with_member{ctor()})->d; + // CHECK: call {{.*}}with_member + // CHECK: call {{.*}}then + then(); + // CHECK: } +} + +// -- a pointer-to-member operation using the .* operator [...] +// CHECK-LABEL: member_ptr_access_1 +void member_ptr_access_1() { + // CHECK: call {{.*}}ctor + auto &&x = with_member{ctor()}.*&with_member::d; + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}with_member + // CHECK: } +} +// CHECK-LABEL: member_ptr_access_2 +void member_ptr_access_2() { + // CHECK: call {{.*}}ctor + auto &&x = (&(const with_member&)with_member{ctor()})->*&with_member::d; + // CHECK: call {{.*}}with_member + // CHECK: call {{.*}}then + then(); + // CHECK: } +} + +// -- a [named] cast [...] +// CHECK-LABEL: static_cast +void test_static_cast() { + // CHECK: call {{.*}}ctor + auto &&x = static_cast(ctor()); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} +// CHECK-LABEL: const_cast +void test_const_cast() { + // CHECK: call {{.*}}ctor + auto &&x = const_cast(ctor()); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} +// CHECK-LABEL: reinterpret_cast +void test_reinterpret_cast() { + // CHECK: call {{.*}}ctor + auto &&x = reinterpret_cast(static_cast(ctor())); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} +// CHECK-LABEL: dynamic_cast +void test_dynamic_cast() { + // CHECK: call {{.*}}ctor + auto &&x = dynamic_cast(ctor()); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} + +// -- [explicit cast notation is defined in terms of the above] +// CHECK-LABEL: c_style_cast +void c_style_cast() { + // CHECK: call {{.*}}ctor + auto &&x = (dtor&&)ctor(); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} +// CHECK-LABEL: function_style_cast +void function_style_cast() { + // CHECK: call {{.*}}ctor + using R = dtor&&; + auto &&x = R(ctor()); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: } +} + +// -- a conditional operator +// CHECK-LABEL: conditional +void conditional(bool b) { + // CHECK: call {{.*}}ctor + // CHECK: call {{.*}}ctor + auto &&x = b ? (dtor&&)ctor() : (dtor&&)ctor(); + // CHECK: call {{.*}}then + then(); + // CHECK: call {{.*}}dtor + // CHECK: call {{.*}}dto
r344801 - PR24164, PR39336: init-captures are not distinct full-expressions.
Author: rsmith Date: Fri Oct 19 12:01:34 2018 New Revision: 344801 URL: http://llvm.org/viewvc/llvm-project?rev=344801&view=rev Log: PR24164, PR39336: init-captures are not distinct full-expressions. Rather, they are subexpressions of the enclosing lambda-expression, and any temporaries in them are destroyed at the end of that full-expression, or when the corresponding lambda-expression is destroyed if they are lifetime-extended. Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/AST/Expr.cpp cfe/trunk/lib/CodeGen/CGExprCXX.cpp cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/lib/Sema/SemaInit.cpp cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/test/CXX/special/class.temporary/p6.cpp cfe/trunk/test/CodeGenCXX/cxx1y-init-captures.cpp cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=344801&r1=344800&r2=344801&view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Fri Oct 19 12:01:34 2018 @@ -5315,8 +5315,7 @@ public: } ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC, bool DiscardedValue = false, - bool IsConstexpr = false, - bool IsLambdaInitCaptureInitializer = false); + bool IsConstexpr = false); StmtResult ActOnFinishFullStmt(Stmt *Stmt); // Marks SS invalid if it represents an incomplete type. Modified: cfe/trunk/lib/AST/Expr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=344801&r1=344800&r2=344801&view=diff == --- cfe/trunk/lib/AST/Expr.cpp (original) +++ cfe/trunk/lib/AST/Expr.cpp Fri Oct 19 12:01:34 2018 @@ -3255,11 +3255,8 @@ bool Expr::HasSideEffects(const ASTConte case LambdaExprClass: { const LambdaExpr *LE = cast(this); -for (LambdaExpr::capture_iterator I = LE->capture_begin(), - E = LE->capture_end(); I != E; ++I) - if (I->getCaptureKind() == LCK_ByCopy) -// FIXME: Only has a side-effect if the variable is volatile or if -// the copy would invoke a non-trivial copy constructor. +for (Expr *E : LE->capture_inits()) + if (E->HasSideEffects(Ctx, IncludePossibleEffects)) return true; return false; } Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=344801&r1=344800&r2=344801&view=diff == --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Fri Oct 19 12:01:34 2018 @@ -2252,7 +2252,6 @@ llvm::Value *CodeGenFunction::EmitDynami } void CodeGenFunction::EmitLambdaExpr(const LambdaExpr *E, AggValueSlot Slot) { - RunCleanupsScope Scope(*this); LValue SlotLV = MakeAddrLValue(Slot.getAddress(), E->getType()); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=344801&r1=344800&r2=344801&view=diff == --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Oct 19 12:01:34 2018 @@ -7764,41 +7764,24 @@ Sema::CorrectDelayedTyposInExpr(Expr *E, ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC, bool DiscardedValue, - bool IsConstexpr, - bool IsLambdaInitCaptureInitializer) { + bool IsConstexpr) { ExprResult FullExpr = FE; if (!FullExpr.get()) return ExprError(); - // If we are an init-expression in a lambdas init-capture, we should not - // diagnose an unexpanded pack now (will be diagnosed once lambda-expr - // containing full-expression is done). - // template void test(Ts ... t) { - // test([&a(t)]() { <-- (t) is an init-expr that shouldn't be diagnosed now. - // return a; - // }() ...); - // } - // FIXME: This is a hack. It would be better if we pushed the lambda scope - // when we parse the lambda introducer, and teach capturing (but not - // unexpanded pack detection) to walk over LambdaScopeInfos which don't have a - // corresponding class yet (that is, have LambdaScopeInfo either represent a - // lambda where we've entered the introducer but not the body, or represent a - // lambda where we've entered the body, depending on where the - // parser/instantiation has got to). - if (!IsLambdaInitCaptureIn
[PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records
gtbercea updated this revision to Diff 170236. gtbercea added a comment. Herald added subscribers: cfe-commits, jholewinski. Refactor. Repository: rC Clang https://reviews.llvm.org/D53141 Files: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp @@ -4238,16 +4238,17 @@ Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF), CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0), S.getIterationVariable()->getType(), S.getBeginLoc()); +return; } + CGOpenMPRuntime::getDefaultDistScheduleAndChunk( + CGF, S, ScheduleKind, Chunk); } void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk( CodeGenFunction &CGF, const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, llvm::Value *&Chunk) const { - if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) { -ScheduleKind = OMPC_SCHEDULE_static; -Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( -S.getIterationVariable()->getType()), 1); - } + ScheduleKind = OMPC_SCHEDULE_static; + Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( + S.getIterationVariable()->getType()), 1); } Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp @@ -4238,16 +4238,17 @@ Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF), CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0), S.getIterationVariable()->getType(), S.getBeginLoc()); +return; } + CGOpenMPRuntime::getDefaultDistScheduleAndChunk( + CGF, S, ScheduleKind, Chunk); } void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk( CodeGenFunction &CGF, const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, llvm::Value *&Chunk) const { - if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) { -ScheduleKind = OMPC_SCHEDULE_static; -Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( -S.getIterationVariable()->getType()), 1); - } + ScheduleKind = OMPC_SCHEDULE_static; + Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( + S.getIterationVariable()->getType()), 1); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.
gtbercea updated this revision to Diff 170237. gtbercea added a comment. Refactor. Repository: rC Clang https://reviews.llvm.org/D53443 Files: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp @@ -4238,16 +4238,17 @@ Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF), CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0), S.getIterationVariable()->getType(), S.getBeginLoc()); +return; } + CGOpenMPRuntime::getDefaultDistScheduleAndChunk( + CGF, S, ScheduleKind, Chunk); } void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk( CodeGenFunction &CGF, const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, llvm::Value *&Chunk) const { - if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) { -ScheduleKind = OMPC_SCHEDULE_static; -Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( -S.getIterationVariable()->getType()), 1); - } + ScheduleKind = OMPC_SCHEDULE_static; + Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( + S.getIterationVariable()->getType()), 1); } Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp === --- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp +++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp @@ -4238,16 +4238,17 @@ Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF), CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0), S.getIterationVariable()->getType(), S.getBeginLoc()); +return; } + CGOpenMPRuntime::getDefaultDistScheduleAndChunk( + CGF, S, ScheduleKind, Chunk); } void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk( CodeGenFunction &CGF, const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind, llvm::Value *&Chunk) const { - if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) { -ScheduleKind = OMPC_SCHEDULE_static; -Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( -S.getIterationVariable()->getType()), 1); - } + ScheduleKind = OMPC_SCHEDULE_static; + Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize( + S.getIterationVariable()->getType()), 1); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records
gtbercea updated this revision to Diff 170238. gtbercea added a comment. Refactor. Repository: rOMP OpenMP https://reviews.llvm.org/D53141 Files: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu libomptarget/deviceRTLs/nvptx/src/interface.h libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu libomptarget/deviceRTLs/nvptx/src/supporti.h Index: libomptarget/deviceRTLs/nvptx/src/supporti.h === --- libomptarget/deviceRTLs/nvptx/src/supporti.h +++ libomptarget/deviceRTLs/nvptx/src/supporti.h @@ -188,7 +188,6 @@ { void *ptr = malloc(size); PRINT(LD_MEM, "malloc data of size %zu for %s: 0x%llx\n", size, msg, P64(ptr)); - ASSERT(LT_SAFETY, ptr, "failed to allocate %zu bytes for %s\n", size, msg); return ptr; } Index: libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu === --- libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu +++ libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu @@ -40,8 +40,6 @@ INLINE unsigned smid() { unsigned id; asm("mov.u32 %0, %%smid;" : "=r"(id)); - ASSERT0(LT_FUSSY, nsmid() <= MAX_SM, - "Expected number of SMs is less than reported."); return id; } @@ -156,7 +154,6 @@ // omptarget_nvptx_TaskDescr *newTaskDescr = omptarget_nvptx_threadPrivateContext->Level1TaskDescr(threadId); - ASSERT0(LT_FUSSY, newTaskDescr, "expected a task descr"); newTaskDescr->InitLevelOneTaskDescr(ThreadLimit, currTeamDescr.LevelZeroTaskDescr()); newTaskDescr->ThreadLimit() = ThreadLimit; Index: libomptarget/deviceRTLs/nvptx/src/interface.h === --- libomptarget/deviceRTLs/nvptx/src/interface.h +++ libomptarget/deviceRTLs/nvptx/src/interface.h @@ -478,6 +478,8 @@ EXTERN void __kmpc_data_sharing_init_stack(); EXTERN void __kmpc_data_sharing_init_stack_spmd(); +EXTERN void *__kmpc_data_sharing_coalesced_push_stack(size_t size, +int16_t UseSharedMemory); EXTERN void *__kmpc_data_sharing_push_stack(size_t size, int16_t UseSharedMemory); EXTERN void __kmpc_data_sharing_pop_stack(void *a); EXTERN void __kmpc_begin_sharing_variables(void ***GlobalArgs, size_t nArgs); Index: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu === --- libomptarget/deviceRTLs/nvptx/src/data_sharing.cu +++ libomptarget/deviceRTLs/nvptx/src/data_sharing.cu @@ -369,96 +369,109 @@ __threadfence_block(); } -// Called at the time of the kernel initialization. This is used to initilize -// the list of references to shared variables and to pre-allocate global storage -// for holding the globalized variables. -// -// By default the globalized variables are stored in global memory. If the -// UseSharedMemory is set to true, the runtime will attempt to use shared memory -// as long as the size requested fits the pre-allocated size. -EXTERN void* __kmpc_data_sharing_push_stack(size_t DataSize, -int16_t UseSharedMemory) { +INLINE void* data_sharing_push_stack_common(size_t PushSize) { if (isRuntimeUninitialized()) { ASSERT0(LT_FUSSY, isSPMDMode(), "Expected SPMD mode with uninitialized runtime."); -return omptarget_nvptx_SimpleThreadPrivateContext::Allocate(DataSize); +return omptarget_nvptx_SimpleThreadPrivateContext::Allocate(PushSize); } + // Only warp active master threads manage the stack. + bool IsWarpMaster = (getThreadId() % WARPSIZE) == 0; + // Add worst-case padding to DataSize so that future stack allocations are // correctly aligned. const size_t Alignment = 8; - if (DataSize % Alignment != 0) { -DataSize += (Alignment - DataSize % Alignment); - } + PushSize = (PushSize + (Alignment - 1)) / Alignment * Alignment; // Frame pointer must be visible to all workers in the same warp. unsigned WID = getWarpId(); + // void * volatile FramePointer = 0; void *&FrameP = DataSharingState.FramePtr[WID]; - // Only warp active master threads manage the stack. - if (getThreadId() % WARPSIZE == 0) { -// SlotP will point to either the shared memory slot or an existing -// global memory slot. -__kmpc_data_sharing_slot *&SlotP = DataSharingState.SlotPtr[WID]; -void *&StackP = DataSharingState.StackPtr[WID]; - -// Compute the total memory footprint of the requested data. -// The master thread requires a stack only for itself. A worker -// thread (which at this point is a warp master) will require -// space for the variables of each thread in the warp, -// i.e. one DataSize chunk per warp lane. -// TODO: change WARPSIZE to the number of active threads in the warp. -size_t PushSize = IsMasterThread() ? DataSize : WARPSIZE * DataSize; + do { +if (IsWarpMaster) { + // SlotP will point to either the shared memory slot or an existing + // global memory slot
r344806 - Revert "[Driver] Reland: Default Android toolchains to libc++."
Author: danalbert Date: Fri Oct 19 12:23:01 2018 New Revision: 344806 URL: http://llvm.org/viewvc/llvm-project?rev=344806&view=rev Log: Revert "[Driver] Reland: Default Android toolchains to libc++." This reverts commit 84677d5009d613232d360fda27e6e41fb5cb6700. Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/test/Driver/android-ndk-standalone.cpp Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=344806&r1=344805&r2=344806&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Fri Oct 19 12:23:01 2018 @@ -443,12 +443,6 @@ Linux::Linux(const Driver &D, const llvm addPathIfExists(D, SysRoot + "/usr/lib", Paths); } -ToolChain::CXXStdlibType Linux::GetDefaultCXXStdlibType() const { - if (getTriple().isAndroid()) -return ToolChain::CST_Libcxx; - return ToolChain::CST_Libstdcxx; -} - bool Linux::HasNativeLLVMSupport() const { return true; } Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); } Modified: cfe/trunk/lib/Driver/ToolChains/Linux.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.h?rev=344806&r1=344805&r2=344806&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Linux.h (original) +++ cfe/trunk/lib/Driver/ToolChains/Linux.h Fri Oct 19 12:23:01 2018 @@ -37,7 +37,6 @@ public: llvm::opt::ArgStringList &CC1Args) const override; void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; - CXXStdlibType GetDefaultCXXStdlibType() const override; bool isPIEDefault() const override; bool IsMathErrnoDefault() const override; SanitizerMask getSupportedSanitizers() const override; Modified: cfe/trunk/test/Driver/android-ndk-standalone.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-ndk-standalone.cpp?rev=344806&r1=344805&r2=344806&view=diff == --- cfe/trunk/test/Driver/android-ndk-standalone.cpp (original) +++ cfe/trunk/test/Driver/android-ndk-standalone.cpp Fri Oct 19 12:23:01 2018 @@ -2,13 +2,21 @@ // toolchain. // // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ -// RUN: -target arm-linux-androideabi21 \ +// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \ // RUN: -B%S/Inputs/basic_android_ndk_tree \ // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \ // RUN: | FileCheck %s // CHECK: {{.*}}clang{{.*}}" "-cc1" // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]" -// CHECK: "-internal-isystem" "{{.*}}/include/c++/v1" +// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9" +// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" +// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a" +// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb" +// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi" +// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" +// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a" +// CHECK-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb" +// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/backward" // CHECK: "-internal-isystem" "{{.*}}/sysroot/usr/local/include" // CHECK: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include" // CHECK: "-internal-externc-isystem" "{{.*}}/sysroot/usr/include/arm-linux-androideabi" @@ -41,47 +49,21 @@ // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi" // // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ -// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \ -// RUN: -B%S/Inputs/basic_android_ndk_tree \ -// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \ -// RUN: | FileCheck --check-prefix=CHECK-STDCXX %s -// CHECK-STDCXX: {{.*}}clang{{.*}}" "-cc1" -// CHECK-STDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]" -// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9" -// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" -// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a" -// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb" -// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi" -// CHECK-STDCXX-NOT: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb" -// C
[PATCH] D53448: [OpenMP][NVPTX] Use single loops when generating code for distribute parallel for
gtbercea created this revision. gtbercea added reviewers: ABataev, caomhin. Herald added subscribers: cfe-commits, guansong, jholewinski. This patch adds a new code generation path for bound sharing directives containing distribute parallel for. The new code generation scheme applies to chunked schedules on distribute and parallel for directives. The scheme simplifies the code that is being generated by eliminating the need for an outer for loop over chunks for both distribute and parallel for directives. In the case of distribute it applies to any sized chunk while in the parallel for case it only applies when chunk size is 1. Repository: rC Clang https://reviews.llvm.org/D53448 Files: include/clang/AST/StmtOpenMP.h include/clang/Basic/OpenMPKinds.h lib/AST/StmtOpenMP.cpp lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp lib/CodeGen/CGStmtOpenMP.cpp lib/Sema/SemaOpenMP.cpp lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp test/OpenMP/distribute_parallel_for_codegen.cpp test/OpenMP/distribute_parallel_for_simd_codegen.cpp Index: test/OpenMP/distribute_parallel_for_simd_codegen.cpp === --- test/OpenMP/distribute_parallel_for_simd_codegen.cpp +++ test/OpenMP/distribute_parallel_for_simd_codegen.cpp @@ -406,18 +406,16 @@ a[i] = b[i] + c[i]; // LAMBDA: define{{.+}} void [[OMP_OUTLINED_3]]( // LAMBDA-DAG: [[OMP_IV:%.omp.iv]] = alloca + // LAMBDA-DAG: [[OMP_CAPT_EXPR:%.capture_expr.1]] = alloca // LAMBDA-DAG: [[OMP_LB:%.omp.comb.lb]] = alloca // LAMBDA-DAG: [[OMP_UB:%.omp.comb.ub]] = alloca // LAMBDA-DAG: [[OMP_ST:%.omp.stride]] = alloca - // unlike the previous tests, in this one we have a outer and inner loop for 'distribute' // LAMBDA: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, i32 91, - // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER:.+]] - // LAMBDA: [[DIST_OUTER_LOOP_HEADER]]: // check EUB for distribute // LAMBDA-DAG: [[OMP_UB_VAL_1:%.+]] = load{{.+}} [[OMP_UB]], - // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}}, + // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}} [[OMP_CAPT_EXPR]], // LAMBDA-DAG: [[CMP_UB_NUM_IT:%.+]] = icmp sgt {{.+}} [[OMP_UB_VAL_1]], [[NUM_IT_1]] // LAMBDA: br {{.+}} [[CMP_UB_NUM_IT]], label %[[EUB_TRUE:.+]], label %[[EUB_FALSE:.+]] // LAMBDA-DAG: [[EUB_TRUE]]: @@ -436,18 +434,9 @@ // check exit condition // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]], - // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_UB]], + // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_CAPT_EXPR]], // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], [[OMP_UB_VAL_3]] - // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_OUTER_LOOP_BODY:.+]], label %[[DIST_OUTER_LOOP_END:.+]] - - // LAMBDA: [[DIST_OUTER_LOOP_BODY]]: - // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER:.+]] - - // LAMBDA: [[DIST_INNER_LOOP_HEADER]]: - // LAMBDA-DAG: [[OMP_IV_VAL_2:%.+]] = load {{.+}} [[OMP_IV]], - // LAMBDA-DAG: [[OMP_UB_VAL_4:%.+]] = load {{.+}} [[OMP_UB]], - // LAMBDA: [[CMP_IV_UB_2:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_2]], [[OMP_UB_VAL_4]] - // LAMBDA: br{{.+}} [[CMP_IV_UB_2]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]] + // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]] // check that PrevLB and PrevUB are passed to the 'for' // LAMBDA: [[DIST_INNER_LOOP_BODY]]: @@ -466,25 +455,39 @@ // LAMBDA-DAG: [[OMP_ST_VAL_1:%.+]] = load {{.+}}, {{.+}}* [[OMP_ST]], // LAMBDA: [[OMP_IV_INC:%.+]] = add{{.+}} [[OMP_IV_VAL_3]], [[OMP_ST_VAL_1]] // LAMBDA: store{{.+}} [[OMP_IV_INC]], {{.+}}* [[OMP_IV]], - // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER]] - - // LAMBDA: [[DIST_INNER_LOOP_END]]: - // LAMBDA: br label %[[DIST_OUTER_LOOP_INC:.+]] - - // LAMBDA: [[DIST_OUTER_LOOP_INC]]: - // check NextLB and NextUB // LAMBDA-DAG: [[OMP_LB_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_LB]], // LAMBDA-DAG: [[OMP_ST_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]], // LAMBDA-DAG: [[OMP_LB_NEXT:%.+]] = add{{.+}} [[OMP_LB_VAL_2]], [[OMP_ST_VAL_2]] // LAMBDA: store{{.+}} [[OMP_LB_NEXT]], {{.+}}* [[OMP_LB]], // LAMBDA-DAG: [[OMP_UB_VAL_5:%.+]] = load{{.+}}, {{.+}} [[OMP_UB]], // LAMBDA-DAG: [[OMP_ST_VAL_3:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]], // LAMBDA-DAG: [[OMP_UB_NEXT:%.+]] = add{{.+}} [[OMP_UB_VAL_5]], [[OMP_ST_VAL_3]] // LAMBDA: store{{.+}} [[OMP_UB_NEXT]], {{.+}}* [[OMP_UB]], - // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER]] - // outer loop exit - // LAMBDA: [[DIST_OUTER_LOOP_END]]: + // Update UB + // LAMBDA-DAG: [[OMP_UB_VAL_6:%.+]] = load{{.+}}, {{.+
[PATCH] D53448: [OpenMP][NVPTX] Use single loops when generating code for distribute parallel for
gtbercea updated this revision to Diff 170241. gtbercea added a comment. Rebase. Repository: rC Clang https://reviews.llvm.org/D53448 Files: include/clang/AST/StmtOpenMP.h include/clang/Basic/OpenMPKinds.h lib/AST/StmtOpenMP.cpp lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp lib/CodeGen/CGOpenMPRuntimeNVPTX.h lib/CodeGen/CGStmtOpenMP.cpp lib/Sema/SemaOpenMP.cpp lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp test/OpenMP/distribute_parallel_for_codegen.cpp test/OpenMP/distribute_parallel_for_simd_codegen.cpp Index: test/OpenMP/distribute_parallel_for_simd_codegen.cpp === --- test/OpenMP/distribute_parallel_for_simd_codegen.cpp +++ test/OpenMP/distribute_parallel_for_simd_codegen.cpp @@ -406,18 +406,16 @@ a[i] = b[i] + c[i]; // LAMBDA: define{{.+}} void [[OMP_OUTLINED_3]]( // LAMBDA-DAG: [[OMP_IV:%.omp.iv]] = alloca + // LAMBDA-DAG: [[OMP_CAPT_EXPR:%.capture_expr.1]] = alloca // LAMBDA-DAG: [[OMP_LB:%.omp.comb.lb]] = alloca // LAMBDA-DAG: [[OMP_UB:%.omp.comb.ub]] = alloca // LAMBDA-DAG: [[OMP_ST:%.omp.stride]] = alloca - // unlike the previous tests, in this one we have a outer and inner loop for 'distribute' // LAMBDA: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, i32 91, - // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER:.+]] - // LAMBDA: [[DIST_OUTER_LOOP_HEADER]]: // check EUB for distribute // LAMBDA-DAG: [[OMP_UB_VAL_1:%.+]] = load{{.+}} [[OMP_UB]], - // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}}, + // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}} [[OMP_CAPT_EXPR]], // LAMBDA-DAG: [[CMP_UB_NUM_IT:%.+]] = icmp sgt {{.+}} [[OMP_UB_VAL_1]], [[NUM_IT_1]] // LAMBDA: br {{.+}} [[CMP_UB_NUM_IT]], label %[[EUB_TRUE:.+]], label %[[EUB_FALSE:.+]] // LAMBDA-DAG: [[EUB_TRUE]]: @@ -436,18 +434,9 @@ // check exit condition // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]], - // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_UB]], + // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_CAPT_EXPR]], // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], [[OMP_UB_VAL_3]] - // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_OUTER_LOOP_BODY:.+]], label %[[DIST_OUTER_LOOP_END:.+]] - - // LAMBDA: [[DIST_OUTER_LOOP_BODY]]: - // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER:.+]] - - // LAMBDA: [[DIST_INNER_LOOP_HEADER]]: - // LAMBDA-DAG: [[OMP_IV_VAL_2:%.+]] = load {{.+}} [[OMP_IV]], - // LAMBDA-DAG: [[OMP_UB_VAL_4:%.+]] = load {{.+}} [[OMP_UB]], - // LAMBDA: [[CMP_IV_UB_2:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_2]], [[OMP_UB_VAL_4]] - // LAMBDA: br{{.+}} [[CMP_IV_UB_2]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]] + // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]] // check that PrevLB and PrevUB are passed to the 'for' // LAMBDA: [[DIST_INNER_LOOP_BODY]]: @@ -466,25 +455,39 @@ // LAMBDA-DAG: [[OMP_ST_VAL_1:%.+]] = load {{.+}}, {{.+}}* [[OMP_ST]], // LAMBDA: [[OMP_IV_INC:%.+]] = add{{.+}} [[OMP_IV_VAL_3]], [[OMP_ST_VAL_1]] // LAMBDA: store{{.+}} [[OMP_IV_INC]], {{.+}}* [[OMP_IV]], - // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER]] - - // LAMBDA: [[DIST_INNER_LOOP_END]]: - // LAMBDA: br label %[[DIST_OUTER_LOOP_INC:.+]] - - // LAMBDA: [[DIST_OUTER_LOOP_INC]]: - // check NextLB and NextUB // LAMBDA-DAG: [[OMP_LB_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_LB]], // LAMBDA-DAG: [[OMP_ST_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]], // LAMBDA-DAG: [[OMP_LB_NEXT:%.+]] = add{{.+}} [[OMP_LB_VAL_2]], [[OMP_ST_VAL_2]] // LAMBDA: store{{.+}} [[OMP_LB_NEXT]], {{.+}}* [[OMP_LB]], // LAMBDA-DAG: [[OMP_UB_VAL_5:%.+]] = load{{.+}}, {{.+}} [[OMP_UB]], // LAMBDA-DAG: [[OMP_ST_VAL_3:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]], // LAMBDA-DAG: [[OMP_UB_NEXT:%.+]] = add{{.+}} [[OMP_UB_VAL_5]], [[OMP_ST_VAL_3]] // LAMBDA: store{{.+}} [[OMP_UB_NEXT]], {{.+}}* [[OMP_UB]], - // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER]] - // outer loop exit - // LAMBDA: [[DIST_OUTER_LOOP_END]]: + // Update UB + // LAMBDA-DAG: [[OMP_UB_VAL_6:%.+]] = load{{.+}}, {{.+}} [[OMP_UB]], + // LAMBDA: [[OMP_EXPR_VAL:%.+]] = load{{.+}}, {{.+}} [[OMP_CAPT_EXPR]], + // LAMBDA-DAG: [[CMP_UB_NUM_IT_1:%.+]] = icmp sgt {{.+}}[[OMP_UB_VAL_6]], [[OMP_EXPR_VAL]] + // LAMBDA: br {{.+}} [[CMP_UB_NUM_IT_1]], label %[[EUB_TRUE_1:.+]], label %[[EUB_FALSE_1:.+]] + // LAMBDA-DAG: [[EUB_TRUE_1]]: + // LAMBDA: [[NUM_IT_3:%.+]] = load{{.+}} [[OMP_CAPT_EXPR]], + // LAMBDA: br label %[[EUB_END_1:.+]] + // LAMBDA-DAG: [[EUB_FALSE_1]]: + // LAMBDA: [[OMP_UB_VAL3:%.+]] =
[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Code looks good to me, but I'd wait for @george.karpenkov because it was his idea. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:502 + + auto AssertM = callExpr(callee(functionDecl(hasName("assert"; + auto GuardM = In a lot of standard libraries `assert()` is implemented as a macro. You might want to catch the corresponding builtin or look at macro names (not sure if we have a matcher for the latter but it should be easy to add). https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error
hubert.reinterpretcast added inline comments. Comment at: clang/test/Sema/altivec-generic-overload.c:3 + +typedef signed char __v4sc __attribute__((__vector_size__(16))); +typedef unsigned char __v4uc __attribute__((__vector_size__(16))); `__v4sc` is suspicious. The most consistent naming I have seen is `v16i8`, `v16u8`, `v8i16`, ... Do we need the double underscores? Repository: rC Clang https://reviews.llvm.org/D53417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:515 + continue; +const auto *FirstAccess = Accesses[0].getNodeAs("access"); + I feel that it's a good practice to `assert(FirstAccess)` here (same for `FirstGuard` and generally after every `getNodeAs`). The assertion will hold because "access" is always bound to on every possible match and is always a member-expression, and it'd be great to check that after the matcher is changed. https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53406: [clangd] Provide excuses for bad code completions, based on diagnostics. C++ API only.
ilya-biryukov added a comment. Since we're showing the diagnostics in the editors anyway, how crucial do we think it is to actually add that to the procotol? Having more concrete reasons for misbehaving completions sounds more useful, though, e.g. "cannot complete members of the incomplete type", etc. Totally fine to have this in the C++ API for experiments, of course. > - LSP provides no protocol mechanims, and editors don't have UI for this Have we tried returning an error in addition to the results? How do the clients behave in that case? Comment at: clangd/CodeComplete.cpp:674 +// Tries to come up with a convincing excuse for our bad completion results, +// based on the diagnostics near the completion point. It took some time to figure out what "excuse" means here. Maybe be more straightforward in naming/comments about what this class does? I.e. finds a last diagnostic on the same line. It's private to this file, so it's easy to rename if we'll want to expand the heuristics. Comment at: clangd/CodeComplete.cpp:686 + for (StartOfLine = Offset; + StartOfLine > 0 && Code[StartOfLine - 1] != '\n'; --StartOfLine) + ; Maybe use a getLineNumber helper from the SourceManager instead? It needs a FileID, but it shouldn't be hard to get one. Comment at: clangd/CodeComplete.cpp:1040 + IncludeStructure *Includes = nullptr, + std::string *Excuse = nullptr) { trace::Span Tracer("Sema completion"); Maybe replace an out parameter with returning a struct of two members? Is there any use for calling code completion without providing an "excuse"? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get
vmiklos created this revision. vmiklos added reviewers: alexfh, sbenza. Herald added subscribers: cfe-commits, xazax.hun. And also enable it by default to be consistent with e.g. modernize-use-using. This helps e.g. when running this check on client code where the macro is provided by the system, so there is no easy way to modify it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53454 Files: clang-tidy/readability/RedundantSmartptrGetCheck.cpp clang-tidy/readability/RedundantSmartptrGetCheck.h docs/clang-tidy/checks/readability-redundant-smartptr-get.rst test/clang-tidy/readability-redundant-smartptr-get-macros.cpp test/clang-tidy/readability-redundant-smartptr-get.cpp Index: test/clang-tidy/readability-redundant-smartptr-get.cpp === --- test/clang-tidy/readability-redundant-smartptr-get.cpp +++ test/clang-tidy/readability-redundant-smartptr-get.cpp @@ -168,6 +168,8 @@ // CHECK-FIXES: if (NULL == x); } +#define MACRO(p) p.get() + void Negative() { struct NegPtr { int* get(); @@ -193,4 +195,7 @@ bool bb = ip.get() == nullptr; bb = !ip.get(); bb = ip.get() ? true : false; + std::unique_ptr x; + if (MACRO(x) == nullptr) +; } Index: test/clang-tidy/readability-redundant-smartptr-get-macros.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-smartptr-get-macros.cpp @@ -0,0 +1,24 @@ +// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t -- \ +// RUN: -config="{CheckOptions: [{key: readability-redundant-smartptr-get.IgnoreMacros, value: 0}]}" \ +// RUN: -- -std=c++11 + +namespace std { + +template +struct shared_ptr { + T &operator*() const; + T *operator->() const; + T *get() const; + explicit operator bool() const noexcept; +}; + +} // namespace std + +#define MACRO(p) p.get() + +void Positive() { + std::shared_ptr x; + if (MACRO(x) == nullptr) +; + // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant get() call on smart pointer +}; Index: docs/clang-tidy/checks/readability-redundant-smartptr-get.rst === --- docs/clang-tidy/checks/readability-redundant-smartptr-get.rst +++ docs/clang-tidy/checks/readability-redundant-smartptr-get.rst @@ -14,3 +14,8 @@ *ptr->get() ==> **ptr if (ptr.get() == nullptr) ... => if (ptr == nullptr) ... + +.. option:: IgnoreMacros + + If this option is set to non-zero (default is `1`), the check will not warn + about calls inside macros. Index: clang-tidy/readability/RedundantSmartptrGetCheck.h === --- clang-tidy/readability/RedundantSmartptrGetCheck.h +++ clang-tidy/readability/RedundantSmartptrGetCheck.h @@ -28,9 +28,14 @@ class RedundantSmartptrGetCheck : public ClangTidyCheck { public: RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), +IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {} + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace readability Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp === --- clang-tidy/readability/RedundantSmartptrGetCheck.cpp +++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp @@ -91,6 +91,11 @@ } // namespace +void RedundantSmartptrGetCheck::storeOptions( +ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) { // Only register the matchers for C++; the functionality currently does not // provide any benefit to other languages, despite being benign. @@ -126,6 +131,9 @@ bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr; bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr; const auto *GetCall = Result.Nodes.getNodeAs("redundant_get"); + if (GetCall->getBeginLoc().isMacroID() && IgnoreMacros) +return; + const auto *Smartptr = Result.Nodes.getNodeAs("smart_pointer"); if (IsPtrToPtr && IsMemberExpr) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get
vmiklos added a comment. In practice cppunit's `CPPUNIT_TEST_SUITE_END` macro triggers this problem. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163 + +switch (FloatingSize) { + case 64: Continuing the float semantics discussion on the new revision - Did you consider `llvm::APFloat`? (http://llvm.org/doxygen/classllvm_1_1APFloat.html) This looks like something that you're trying to re-implement. Repository: rC Clang https://reviews.llvm.org/D52730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used
echristo added a reviewer: aprantl. echristo added a comment. I think Adrian has looked at this more recently than I have. Adding him here. Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1268806, @djasper wrote: > In https://reviews.llvm.org/D52676#1268748, @oleg.smolsky wrote: > > > In https://reviews.llvm.org/D52676#1268706, @djasper wrote: > > > > > Ok, I think I agree with both of you to a certain extent, but I also > > > think this change as is, is not yet right. > > > > > > First, it does too much. The original very first example in this CL is > > > actually not produced by clang-format (or if it is, I don't know with > > > which flag combination). It is a case where the lambda is the last > > > parameter. > > > > > > Right, I cheated and created that example by hand. My apologies for the > > confusion. I've just pasted real code that I pumped through `clang-format`. > > Please take a look at the updated summary. > > > > > Second, I agree that the original behavior is inconsistent in a way that > > > we have a special cases for when the lambda is the very first parameter, > > > but somehow seem be forgetting about that when it's not the first > > > parameter. I'd be ok with "fixing" that (it's not a clear-cut bug, but I > > > think the alternative behavior would be cleaner). However, I don't think > > > your patch is doing enough there. I think this should be irrespective of > > > bin-packing (it's related but not quite the same thing), > > > > Also there is a special case for multiple lambdas. It forces line breaks. > > That aside, for the single-lambda case, are you suggesting that it is > > always "pulled up", irrespective of its place? That would contradict the > > direction I am trying to take as I like `BinPackArguments: false` and so > > long lamba args go to their own lines. This looks very much in line with > > what bin packing is, but not exactly the same. Obviously, I can add a new > > option `favor line breaks around multi-line lambda`. > > > I don't think I am. You are right, there is the special case for multi-lambda > functions and I think we should have almost the same for single-lambda > functions. So, I think I agree with you and am in favor of: > > someFunction( > a, > [] { > // ... > }, > b); > > > And this is irrespective of BinPacking. I think this is always better and > more consistent with what we'd be doing if "a" was not there. The only caveat > is that I think with BinPacking true or false, we should keep the more > compact formatting if "b" isn't there and the lambda introducer fits entirely > on the first line: > > someFunction(a, [] { > // ... > }); OK, cool, I've generalized the patch and extended the test. This global thing also effects other existing test that deal with sub-blocks/lambdas. >> Could you look at the updated summary (high level) and the new tests I added >> (low level) please? Every other test passes, so we have almost the entire >> spec. I can add a few cases where an existing snippet is reformatted with >> `BinPackArguments: false` too. > > Not sure I am seeing new test cases and I think at least a few cases are > missing from the entire spec, e.g. the case above. > > Also, try to reduce the test cases a bit more: > > - They don't need the surrounding functions > - You can force the lambda to be multi-line with a "//" comment > - There is no reason to have the lambda be an argument to a member function, > a free-standing function works just as well > > This might seem nit-picky, but in my experience, the more we can reduce the > test cases, the easier to read and the less brittle they become. Makes sense, reduced the extra levels. >>> ...and it should also apply to multiline strings if >>> AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done >>> in the same patch, but we should have a plan of what we want the end result >>> to look like and should be willing to get there. Oh, yeah, I will look at that next. Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53456: [Sema] Do not show unused parameter warnings when body is skipped
ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added a subscriber: arphaman. Without the function body, we cannot determine is parameter was used. Repository: rC Clang https://reviews.llvm.org/D53456 Files: lib/Sema/SemaDecl.cpp test/Index/skipped-bodies-unused.cpp Index: test/Index/skipped-bodies-unused.cpp === --- /dev/null +++ test/Index/skipped-bodies-unused.cpp @@ -0,0 +1,8 @@ +// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s -Wunused-parameter 2>&1 \ +// RUN: | FileCheck %s + +// No 'unused parameter' warnings should be shown when skipping the function bodies. +inline int foo(int used, int unused) { +used = 100; +} +// CHECK-NOT: warning: unused parameter Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -13037,7 +13037,7 @@ if (!FD->isInvalidDecl()) { // Don't diagnose unused parameters of defaulted or deleted functions. - if (!FD->isDeleted() && !FD->isDefaulted()) + if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody()) DiagnoseUnusedParameters(FD->parameters()); DiagnoseSizeOfParametersAndReturnValue(FD->parameters(), FD->getReturnType(), FD); @@ -13132,7 +13132,8 @@ assert(MD == getCurMethodDecl() && "Method parsing confused"); MD->setBody(Body); if (!MD->isInvalidDecl()) { - DiagnoseUnusedParameters(MD->parameters()); + if (!MD->hasSkippedBody()) +DiagnoseUnusedParameters(MD->parameters()); DiagnoseSizeOfParametersAndReturnValue(MD->parameters(), MD->getReturnType(), MD); Index: test/Index/skipped-bodies-unused.cpp === --- /dev/null +++ test/Index/skipped-bodies-unused.cpp @@ -0,0 +1,8 @@ +// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s -Wunused-parameter 2>&1 \ +// RUN: | FileCheck %s + +// No 'unused parameter' warnings should be shown when skipping the function bodies. +inline int foo(int used, int unused) { +used = 100; +} +// CHECK-NOT: warning: unused parameter Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -13037,7 +13037,7 @@ if (!FD->isInvalidDecl()) { // Don't diagnose unused parameters of defaulted or deleted functions. - if (!FD->isDeleted() && !FD->isDefaulted()) + if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody()) DiagnoseUnusedParameters(FD->parameters()); DiagnoseSizeOfParametersAndReturnValue(FD->parameters(), FD->getReturnType(), FD); @@ -13132,7 +13132,8 @@ assert(MD == getCurMethodDecl() && "Method parsing confused"); MD->setBody(Body); if (!MD->isInvalidDecl()) { - DiagnoseUnusedParameters(MD->parameters()); + if (!MD->hasSkippedBody()) +DiagnoseUnusedParameters(MD->parameters()); DiagnoseSizeOfParametersAndReturnValue(MD->parameters(), MD->getReturnType(), MD); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky updated this revision to Diff 170270. oleg.smolsky added a comment. Generalized the patch so that it deals with lambda args irrespective of bin packing. Added additional tests and patched existing ones. Repository: rC Clang https://reviews.llvm.org/D52676 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -3178,25 +3178,27 @@ "});"); FormatStyle Style = getGoogleStyle(); Style.ColumnLimit = 45; - verifyFormat("Debug(a,\n" - " {\n" - "if () return;\n" - " },\n" - " a);", + verifyFormat("Debug(\n" + "a,\n" + "{\n" + " if () return;\n" + "},\n" + "a);", Style); verifyFormat("SomeFunction({MACRO({ return output; }), b});"); verifyNoCrash("^{v^{a}}"); } TEST_F(FormatTest, FormatNestedBlocksInMacros) { - EXPECT_EQ("#define MACRO() \\\n" -" Debug(aaa, /* force line break */ \\\n" -"{ \\\n" -" int i;\\\n" -" int j;\\\n" -"})", + EXPECT_EQ("#define MACRO() \\\n" +" Debug( \\\n" +" aaa, /* force line break */ \\\n" +" { \\\n" +"int i;\\\n" +"int j;\\\n" +" })", format("#define MACRO() Debug(aaa, /* force line break */ \\\n" " { int i; int j; })", getGoogleStyle())); @@ -11596,9 +11598,10 @@ " other(x.begin(), x.end(), [&](int, int) { return 1; });\n" "}\n"); verifyFormat("void f() {\n" - " other(x.begin(), //\n" - "x.end(), //\n" - "[&](int, int) { return 1; });\n" + " other(\n" + " x.begin(), //\n" + " x.end(), //\n" + " [&](int, int) { return 1; });\n" "}\n"); verifyFormat("SomeFunction([]() { // A cool function...\n" " return 43;\n" @@ -11651,9 +11654,9 @@ verifyFormat("SomeFunction({[&] {\n" " // comment\n" "}});"); - verifyFormat("virtual (std::function =\n" - " [&]() { return true; },\n" - " a a);"); + verifyFormat("virtual (\n" + "std::function = [&]() { return true; },\n" + "a a);"); // Lambdas with return types. verifyFormat("int c = []() -> int { return 2; }();\n"); @@ -11680,17 +11683,74 @@ " return 1; //\n" "};"); - // Multiple lambdas in the same parentheses change indentation rules. + // Multiple lambdas in the same parentheses change indentation rules. These + // lambdas are forced to start on new lines. verifyFormat("SomeFunction(\n" "[]() {\n" - " int i = 42;\n" - " return i;\n" + " //\n" "},\n" "[]() {\n" - " int j = 43;\n" - " return j;\n" + " //\n" "});"); + // A lambda passed as arg0 is always pushed to the next line. + verifyFormat("SomeFunction(\n" + "[this] {\n" + " //\n" + "},\n" + "1);\n"); + + // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0 + // case above. + { +auto Style = getGoogleStyle(); +Style.BinPackArguments = false; +verifyFormat("SomeFunction(\n" + "a,\n" + "[this] {\n" + " //\n" + "},\n" + "b);\n", + Style); + } + { +verifyFormat("SomeFunction(\n" + "a,\n" + "[this] {\n" + " //\n" + "},\n" + "b);\n"); + } + + // A lambda with a very long line forces arg0 to be pushed out irrespective of + // the BinPackArguments value (as long as the code is wide enough). + verifyFormat("something->SomeFu
[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.
neerajksingh created this revision. neerajksingh added reviewers: rnk, hans. The clang-cl driver disables access to command line options outside of the "Core" and "CLOption" sets of command line arguments. This filtering makes it impossible to pass arguments that are interpreted by the clang driver and not by either 'cc1' (the frontend) or the LLVM backend. An example driver-level flag is the '-fno-slp-vectorize' flag, which is processed by the driver in Clang::ConstructJob and used to set the cc1 flag "-vectorize-slp". There is no negative cc1 flag or -mllvm flag, so it is not currently possible to disable the SLP vectorizer from the clang-cl driver. This change introduces the "/Xdriver:" argument that is available when the driver mode is set to CL compatibility. This option works similarly to the "-Xclang" option, except that the option values are processed by the clang driver rather than by 'cc1'. An example usage is: clang-cl /Xdriver:-fno-slp-vectorize /O2 test.c Another example shows how "/Xdriver:" can be used to pass a flag where there is a conflict between a clang-cl compat option and an overlapping clang driver option: clang-cl /MD /Xdriver:-MD /Xdriver:-MF /Xdriver:test_dep_file.dep test.c In the previous example, the unprefixed /MD selects the DLL version of the msvc CRT, while the prefixed -MD flag and the -MF flags are used to create a make dependency file for included headers. One note about flag ordering: the /Xdriver: flags are concatenated to the end of the argument list, so in cases where the last flag wins, the /Xdriver: flags will be chosen regardless of their order relative to other flags on the driver command line. Repository: rC Clang https://reviews.llvm.org/D53457 Files: include/clang/Driver/CLCompatOptions.td include/clang/Driver/Driver.h lib/Driver/Driver.cpp test/Driver/cl-options.c Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -614,5 +614,17 @@ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 +// Accept clang options under the /Xdriver: flag. + +// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOXDRIVER %s +// NOXDRIVER: "--dependent-lib=libcmt" +// NOXDRIVER-SAME: "-vectorize-slp" +// NOXDRIVER-NOT: "--dependent-lib=msvcrt" + +// RUN: %clang_cl -O2 -MD /Xdriver:-fno-slp-vectorize /Xdriver:-MD /Xdriver:-MF /Xdriver:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=XDRIVER %s +// XDRIVER: "--dependent-lib=msvcrt" +// XDRIVER-SAME: "-dependency-file" "my_dependency_file.dep" +// XDRIVER-NOT: "--dependent-lib=libcmt" +// XDRIVER-NOT: "-vectorize-slp" void f() { } Index: lib/Driver/Driver.cpp === --- lib/Driver/Driver.cpp +++ lib/Driver/Driver.cpp @@ -166,14 +166,15 @@ } InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings, + bool ForceNonCLMode, bool &ContainsError) { llvm::PrettyStackTraceString CrashInfo("Command line argument parsing"); ContainsError = false; unsigned IncludedFlagsBitmask; unsigned ExcludedFlagsBitmask; std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) = - getIncludeExcludeOptionFlagMasks(); + getIncludeExcludeOptionFlagMasks(ForceNonCLMode); unsigned MissingArgIndex, MissingArgCount; InputArgList Args = @@ -730,7 +731,7 @@ ConfigFile = CfgFileName.str(); bool ContainErrors; CfgOptions = llvm::make_unique( - ParseArgStrings(NewCfgArgs, ContainErrors)); + ParseArgStrings(NewCfgArgs, false, ContainErrors)); if (ContainErrors) { CfgOptions.reset(); return true; @@ -924,7 +925,7 @@ // Arguments specified in command line. bool ContainsError; CLOptions = llvm::make_unique( - ParseArgStrings(ArgList.slice(1), ContainsError)); + ParseArgStrings(ArgList.slice(1), false, ContainsError)); // Try parsing configuration file. if (!ContainsError) @@ -934,21 +935,45 @@ // All arguments, from both config file and command line. InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions) : std::move(*CLOptions)); - if (HasConfigFile) -for (auto *Opt : *CLOptions) { - if (Opt->getOption().matches(options::OPT_config)) -continue; + + auto appendOneArg = [&Args](const Arg *Opt, const Arg *BaseArg) { unsigned Index = Args.MakeIndex(Opt->getSpelling()); - const Arg *BaseArg = &Opt->getBaseArg(); - if (BaseArg == Opt) -BaseArg = nullptr; Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(), Index, BaseArg); Copy->getValues() = Opt->getValues(); if (Opt->isClaimed()) Copy->claim(); Args.append(Copy); + }; + + if (HasConfigFile) +for (auto *Opt : *CLOptions) { +
[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.
rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:711 + + /// Evaluate as a constant expression, as per C++11-and-later constexpr + /// rules. Stop if we find that the expression is not a constant jyknight wrote: > rsmith wrote: > > This is not the intent. The evaluator uses the rules of the current > > language mode. However, it doesn't enforce the C and C++98 syntactic > > restrictions on what can appear in a constant expression, because it turns > > out to be cleaner to check those separately rather than to interleave them > > with evaluation. > > > > We should probably document this as evaluating following the evaluation > > (but not syntactic) constant expression rules of the current language mode. > I'm not really sure what you mean to distinguish between C/C++98 "evaluation > constant expression rules" and the definition of a C/C++98 ICE? > > Perhaps you mean that if evaluation succeeds, it will have produced a value > consistent with the rules of the current language, but that it may not > produce the diagnostics in all cases it ought to? > > AFAICT, the evaluator is designed to emit diagnostics only for the C++11 (and > later) constant expression rules, as well as for inability to evaluate. The > diagnostic notes are usually dropped on the floor in C++98 and C modes, as > only "fold" is typically used there -- along with the separate CheckICE to > produce warnings. > > I believe the only place the diagnostics were being actually emitted in > pre-c++11 modes is for the diagnose_if and enable_if attributes' "potential > constant expression" check. And there it really seems to me to be using the > c++11 evaluation semantics. > > The existence of isCXX11ConstantExpr suggests an intent for the function to > provide the C++11 rules, as well, no? > > > Perhaps you mean that if evaluation succeeds, it will have produced a value > consistent with the rules of the current language, but that it may not > produce the diagnostics in all cases it ought to? The concern I have with this comment is that it describes a happenstance of implementation rather than the intent. The intent is certainly *not* that we will use the C++11 rules when calling this in C++98 mode, but it just so happens that there are no interesting differences there right now. Looking at this another way, the fact that we only guarantee to produce "not a constant expression because" notes in C++11 onwards is unrelated to the evaluation mode. Rather, the reason for that is instead because all the validity / ICE checking in other language modes is based on the syntactic form of the expression (and is checked separately as a result). As a concrete suggestion, how about something like: "Evaluate as a constant expression. Stop if we find that the expression is not a constant expression. Note that this doesn't enforce the syntactic ICE requirements of C and C++98." Comment at: clang/lib/AST/ExprConstant.cpp:952 +/// with diagnostics. +bool keepEvaluatingAfterNote() { switch (EvalMode) { jyknight wrote: > rsmith wrote: > > "Note" is beside the point here. We should be talking about non-constant > > expressions; that's what matters. > Renamed to keepEvaluatingAfterNonConstexpr, updated comment. Capitalize as "...NonConstExpr" instead, since this is just an abbreviation of the term "constant expression", not C++11's notion of "constexpr". (I'd also lengthen this to "...NonConstantExpr" or "...NonConstantExpression" to further clarify.) Comment at: clang/lib/AST/ExprConstant.cpp:1440-1444 + if (checkSubobject(Info, E, isa(D) ? CSK_Field : CSK_Base)) { Designator.addDeclUnchecked(D, Virtual); +return true; + } + return Info.keepEvaluatingAfterNote(); jyknight wrote: > rsmith wrote: > > `checkSubobject` should return `false` if it produced a diagnostic that > > should cause evaluation to halt; this call to `keepEvaluatingAfterNote` > > should not be necessary. (And likewise below.) > Are you sure? > > Today, checkSubobject returns false if it sees something "invalid" -- and the > callers seem to depend on that -- but invalid subobject designators are still > allowed during evaluation in fold mode. > > E.g., this is valid in C: > `struct Foo { int x[]; }; struct Foo f; int *g = f.x;` > > But, in C++, `constexpr int *g = f.x;` is an error, diagnosing > note_constexpr_unsupported_unsized_array. > > All the LValue code, with its mutable "Result" state, and carrying of a > couple different "Invalid" bits off on the side, is really rather confusing! What I mean is that you should change the code to make what I said be true, if necessary. It's the callee's responsibility to say whether we hit something we can't evaluate past. The caller should not be guessing that the only reason that `checkSubobject` can fail is due to a `C
[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used
aprantl added a comment. I have a vague recollection that this column info hack was added to disambiguate two types defined on the same line (which is something that happened more often than one would think because of macro expansion). Did you do the git archeology to ensure that the original reason for doing it this way has been resolved? FWIW, I'm fine with doing this change for the Darwin platforms because column info is turned on by default, so this shouldn't affect us. Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used
aprantl added a comment. Oh wait, this patch is just for dumping the ASTs? Can you elaborate why this makes it into a binary then? Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used
aprantl added a comment. RTTI? Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53347: [clangd] Simplify auto hover
ilya-biryukov updated this revision to Diff 170273. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Addressed review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53347 Files: clangd/XRefs.cpp unittests/clangd/XRefsTests.cpp Index: unittests/clangd/XRefsTests.cpp === --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -999,6 +999,13 @@ )cpp", "", }, + { + R"cpp(// More compilcated structured types. +int bar(); +^auto (*foo)() = bar; + )cpp", + "int", + }, }; for (const OneTest &Test : Tests) { Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -585,17 +585,6 @@ llvm::Optional getDeducedType() { return DeducedType; } - // Remove the surrounding Reference or Pointer type of the given type T. - QualType UnwrapReferenceOrPointer(QualType T) { -// "auto &" is represented as a ReferenceType containing an AutoType -if (const ReferenceType *RT = dyn_cast(T.getTypePtr())) - return RT->getPointeeType(); -// "auto *" is represented as a PointerType containing an AutoType -if (const PointerType *PT = dyn_cast(T.getTypePtr())) - return PT->getPointeeType(); -return T; - } - // Handle auto initializers: //- auto i = 1; //- decltype(auto) i = 1; @@ -606,17 +595,9 @@ D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation) return true; -auto DeclT = UnwrapReferenceOrPointer(D->getType()); -const AutoType *AT = dyn_cast(DeclT.getTypePtr()); -if (AT && !AT->getDeducedType().isNull()) { - // For auto, use the underlying type because the const& would be - // represented twice: written in the code and in the hover. - // Example: "const auto I = 1", we only want "int" when hovering on auto, - // not "const int". - // - // For decltype(auto), take the type as is because it cannot be written - // with qualifiers or references but its decuded type can be const-ref. - DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType(); +if (auto *AT = D->getType()->getContainedAutoType()) { + if (!AT->getDeducedType().isNull()) +DeducedType = AT->getDeducedType(); } return true; } @@ -640,12 +621,13 @@ if (CurLoc != SearchedLocation) return true; -auto T = UnwrapReferenceOrPointer(D->getReturnType()); -const AutoType *AT = dyn_cast(T.getTypePtr()); +const AutoType *AT = D->getReturnType()->getContainedAutoType(); if (AT && !AT->getDeducedType().isNull()) { - DeducedType = T.getUnqualifiedType(); -} else { // auto in a trailing return type just points to a DecltypeType. - const DecltypeType *DT = dyn_cast(T.getTypePtr()); + DeducedType = AT->getDeducedType(); +} else { + // auto in a trailing return type just points to a DecltypeType and + // getContainedAutoType does not unwrap it. + const DecltypeType *DT = dyn_cast(D->getReturnType()); if (!DT->getUnderlyingType().isNull()) DeducedType = DT->getUnderlyingType(); } Index: unittests/clangd/XRefsTests.cpp === --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -999,6 +999,13 @@ )cpp", "", }, + { + R"cpp(// More compilcated structured types. +int bar(); +^auto (*foo)() = bar; + )cpp", + "int", + }, }; for (const OneTest &Test : Tests) { Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -585,17 +585,6 @@ llvm::Optional getDeducedType() { return DeducedType; } - // Remove the surrounding Reference or Pointer type of the given type T. - QualType UnwrapReferenceOrPointer(QualType T) { -// "auto &" is represented as a ReferenceType containing an AutoType -if (const ReferenceType *RT = dyn_cast(T.getTypePtr())) - return RT->getPointeeType(); -// "auto *" is represented as a PointerType containing an AutoType -if (const PointerType *PT = dyn_cast(T.getTypePtr())) - return PT->getPointeeType(); -return T; - } - // Handle auto initializers: //- auto i = 1; //- decltype(auto) i = 1; @@ -606,17 +595,9 @@ D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation) return true; -auto DeclT = UnwrapReferenceOrPointer(D->getType()); -const AutoType *AT = dyn_cast(DeclT.getTypePtr()); -if (AT && !AT->getDeducedType().isNull()) { - // For auto, use the underlying type because the const& would be - // represented twice: written in the code and in
Re: [PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used
On Fri, Oct 19, 2018 at 3:56 PM Adrian Prantl via Phabricator via llvm-commits wrote: > aprantl added a comment. > > I have a vague recollection that this column info hack was added to > disambiguate two types defined on the same line (which is something that > happened more often than one would think because of macro expansion). Did > you do the git archeology to ensure that the original reason for doing it > this way has been resolved? FWIW, I'm fine with doing this change for the > Darwin platforms because column info is turned on by default, so this > shouldn't affect us. > It was (I think) duplicate calls (rather than types) - so that inlining (which previously didn't produce a unique'd inlinedAt location (because we didn't have the "unique" metadata feature)) didn't collapse the instructions from the two inlined calls into one. > > > Repository: > rC Clang > > https://reviews.llvm.org/D38061 > > > > ___ > llvm-commits mailing list > llvm-comm...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. (Looks fine with a suitably-adjusted comment.) https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
rsmith added inline comments. Comment at: lib/AST/DeclBase.cpp:1704-1705 + + // In C, the redeclaration context for enumerators is the translation unit, + // so we skip through transparent contexts as well as struct/union contexts. + bool SkipRecords = getDeclKind() == Decl::Kind::Enum && Nit: "the redeclaration context for enumerators is the translation unit" is not entirely accurate. The point instead is that a record type is only the redeclaration context for the fields of that record, so if we arrive at that context after skipping anything else, we should skip the record as well. (The check for "Enum" here is a red herring in that regard, but it happens to be correct because enumerations are the only transparent context that can exist within a struct or union currently.) https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53347: [clangd] Simplify auto hover
ilya-biryukov added a comment. In https://reviews.llvm.org/D53347#1267216, @kadircet wrote: > LGTM, it bugs me that some part in the documentation says it doesn't go > through decltype(`This looks through declarators like pointer types, but not > through decltype or typedefs`) but since tests containing decltype didn't > break it looks OK to me. I would still wait for @hokein to have another look > as well. The decltypes are handled separately in `VisitDecltypeTypeLoc` and typedefs are a decl, so they are handled even before the visitor runs. This is only about 'auto'. So I think we're covered here. Comment at: clangd/XRefs.cpp:592 -auto DeclT = UnwrapReferenceOrPointer(D->getType()); -const AutoType *AT = dyn_cast(DeclT.getTypePtr()); -if (AT && !AT->getDeducedType().isNull()) { - // For auto, use the underlying type because the const& would be - // represented twice: written in the code and in the hover. - // Example: "const auto I = 1", we only want "int" when hovering on auto, - // not "const int". - // - // For decltype(auto), take the type as is because it cannot be written - // with qualifiers or references but its decuded type can be const-ref. - DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType(); -} +auto AT = D->getType()->getContainedAutoType(); +if (AT && !AT->getDeducedType().isNull()) hokein wrote: > Maybe just > > ``` > if (auto* DT = D->getType()->getContainedDeducedType()) >DeducedType = *DT; > ``` > > ? It feels like explicitly unwrapping auto from the deduced type instead of relying the printer to do that is a bit more straightforward. Simplified code a bit per suggestions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.
hans added a comment. I haven't started looking at the code yet. I'm not completely convinced that we want this. So far we've used the strategy of promoting clang options that are also useful in clang-cl to core options, and if someone wants to use more clang than that, maybe clang-cl isn't the right driver for that use-case. But I suppose an argument could be made for having an escape hatch from clang-cl if it doesn't add too much complexity to the code. I'm not sure I'm a fan of calling it /Xdriver: though, because what does it mean - clang-cl is the driver, but the option refers to the clang driver. The natural name would of course be -Xclang but that already means something else. Maybe we could just call it /clang: Repository: rC Clang https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location
aprantl created this revision. aprantl added a reviewer: vsk. Function calls without a !dbg location inside a function that has a DISubprogram make it impossible to construct inline information and are rejected by the verifier. This patch ensures that sanitizer check function calls have a !dbg location, by carrying forward the location of the preceding instruction or by inserting an artificial location if necessary. This fixes a crash when compiling the attached testcase with -Os. rdar://problem/45311226 https://reviews.llvm.org/D53459 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/ubsan-check-debuglocs.cpp Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-check-debuglocs.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \ +// RUN: -fsanitize=null,object-size,return -fsanitize-recover=null \ +// RUN: %s -o - | FileCheck %s + +// Check that santizer check calls have a !dbg location. +// CHECK: define {{.*}}acquire{{.*}} !dbg +// CHECK-NOT: define +// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1 {{.*}}, !dbg + +class SourceLocation { +public: + SourceLocation acquire() {}; +}; +extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc); +static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); } +void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) { + handleTypeMismatchImpl(Loc); +} Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -2867,6 +2867,9 @@ CheckRecoverableKind RecoverKind, bool IsFatal, llvm::BasicBlock *ContBB) { assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable); + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); bool NeedsAbortSuffix = IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable; bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime; Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-check-debuglocs.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \ +// RUN: -fsanitize=null,object-size,return -fsanitize-recover=null \ +// RUN: %s -o - | FileCheck %s + +// Check that santizer check calls have a !dbg location. +// CHECK: define {{.*}}acquire{{.*}} !dbg +// CHECK-NOT: define +// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1 {{.*}}, !dbg + +class SourceLocation { +public: + SourceLocation acquire() {}; +}; +extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc); +static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); } +void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) { + handleTypeMismatchImpl(Loc); +} Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -2867,6 +2867,9 @@ CheckRecoverableKind RecoverKind, bool IsFatal, llvm::BasicBlock *ContBB) { assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable); + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); bool NeedsAbortSuffix = IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable; bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53460: [X86] When checking the bits in cpu_features for function multiversioning dispatcher in the resolver, make sure all the required bits are set. Not just one of them
craig.topper created this revision. craig.topper added reviewers: erichkeane, echristo. The multiversioning code repurposed the code from __builtin_cpu_supports for checking if a single feature is enabled. That code essentially performed (_cpu_features & (1 << C)) != 0. But with the multiversioning path, the mask is no longer guaranteed to be a power of 2. So we return true anytime any one of the bits in the mask is set not just all of the bits. The correct check is (_cpu_features & mask) == mask https://reviews.llvm.org/D53460 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGen/attr-target-mv.c test/CodeGen/builtin-cpu-supports.c Index: test/CodeGen/builtin-cpu-supports.c === --- test/CodeGen/builtin-cpu-supports.c +++ test/CodeGen/builtin-cpu-supports.c @@ -14,7 +14,7 @@ // CHECK: [[LOAD:%[^ ]+]] = load i32, i32* getelementptr inbounds ({ i32, i32, i32, [1 x i32] }, { i32, i32, i32, [1 x i32] }* @__cpu_model, i32 0, i32 3, i32 0) // CHECK: [[AND:%[^ ]+]] = and i32 [[LOAD]], 256 - // CHECK: = icmp ne i32 [[AND]], 0 + // CHECK: = icmp eq i32 [[AND]], 256 return 0; } Index: test/CodeGen/attr-target-mv.c === --- test/CodeGen/attr-target-mv.c +++ test/CodeGen/attr-target-mv.c @@ -70,6 +70,22 @@ // CHECK: ret void ()* @foo_decls.sse4.2 // CHECK: ret void ()* @foo_decls +// CHECK: define void @bar4() +// CHECK: call void @foo_multi.ifunc() + +// CHECK: define void ()* @foo_multi.resolver() comdat +// CHECK: and i32 %{{.*}}, 4352 +// CHECK: icmp eq i32 %{{.*}}, 4352 +// CHECK: ret void ()* @foo_multi.fma4_sse4.2 +// CHECK: icmp eq i32 %{{.*}}, 12 +// CHECK: and i32 %{{.*}}, 4352 +// CHECK: icmp eq i32 %{{.*}}, 4352 +// CHECK: ret void ()* @foo_multi.arch_ivybridge_fma4_sse4.2 +// CHECK: and i32 %{{.*}}, 768 +// CHECK: icmp eq i32 %{{.*}}, 768 +// CHECK: ret void ()* @foo_multi.avx_sse4.2 +// CHECK: ret void ()* @foo_multi + // CHECK: declare i32 @foo.arch_sandybridge() // CHECK: define available_externally i32 @foo_inline.sse4.2() @@ -88,4 +104,3 @@ // CHECK: define available_externally void @foo_multi.avx_sse4.2() // CHECK: define available_externally void @foo_multi.fma4_sse4.2() // CHECK: define available_externally void @foo_multi.arch_ivybridge_fma4_sse4.2() - Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -9129,9 +9129,9 @@ Builder.CreateAlignedLoad(CpuFeatures, CharUnits::fromQuantity(4)); // Check the value of the bit corresponding to the feature requested. - Value *Bitset = Builder.CreateAnd( - Features, llvm::ConstantInt::get(Int32Ty, FeaturesMask)); - return Builder.CreateICmpNE(Bitset, llvm::ConstantInt::get(Int32Ty, 0)); + Value *Mask = Builder.getInt32(FeaturesMask); + Value *Bitset = Builder.CreateAnd(Features, Mask); + return Builder.CreateICmpEQ(Bitset, Mask); } Value *CodeGenFunction::EmitX86CpuInit() { Index: test/CodeGen/builtin-cpu-supports.c === --- test/CodeGen/builtin-cpu-supports.c +++ test/CodeGen/builtin-cpu-supports.c @@ -14,7 +14,7 @@ // CHECK: [[LOAD:%[^ ]+]] = load i32, i32* getelementptr inbounds ({ i32, i32, i32, [1 x i32] }, { i32, i32, i32, [1 x i32] }* @__cpu_model, i32 0, i32 3, i32 0) // CHECK: [[AND:%[^ ]+]] = and i32 [[LOAD]], 256 - // CHECK: = icmp ne i32 [[AND]], 0 + // CHECK: = icmp eq i32 [[AND]], 256 return 0; } Index: test/CodeGen/attr-target-mv.c === --- test/CodeGen/attr-target-mv.c +++ test/CodeGen/attr-target-mv.c @@ -70,6 +70,22 @@ // CHECK: ret void ()* @foo_decls.sse4.2 // CHECK: ret void ()* @foo_decls +// CHECK: define void @bar4() +// CHECK: call void @foo_multi.ifunc() + +// CHECK: define void ()* @foo_multi.resolver() comdat +// CHECK: and i32 %{{.*}}, 4352 +// CHECK: icmp eq i32 %{{.*}}, 4352 +// CHECK: ret void ()* @foo_multi.fma4_sse4.2 +// CHECK: icmp eq i32 %{{.*}}, 12 +// CHECK: and i32 %{{.*}}, 4352 +// CHECK: icmp eq i32 %{{.*}}, 4352 +// CHECK: ret void ()* @foo_multi.arch_ivybridge_fma4_sse4.2 +// CHECK: and i32 %{{.*}}, 768 +// CHECK: icmp eq i32 %{{.*}}, 768 +// CHECK: ret void ()* @foo_multi.avx_sse4.2 +// CHECK: ret void ()* @foo_multi + // CHECK: declare i32 @foo.arch_sandybridge() // CHECK: define available_externally i32 @foo_inline.sse4.2() @@ -88,4 +104,3 @@ // CHECK: define available_externally void @foo_multi.avx_sse4.2() // CHECK: define available_externally void @foo_multi.fma4_sse4.2() // CHECK: define available_externally void @foo_multi.arch_ivybridge_fma4_sse4.2() - Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -9129,9 +9129,9 @@ Builder.CreateAl
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
BillyONeal closed this revision. BillyONeal added a comment. Committed r344820 https://reviews.llvm.org/D50549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50551: [libcxx] [test] Add missing to several tests.
BillyONeal closed this revision. BillyONeal added a comment. Committed r344821 https://reviews.llvm.org/D50551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type
bobsayshilol updated this revision to Diff 170277. bobsayshilol retitled this revision from "[CodeGen] Fix places where the return type of a FunctionDecl was being used in place of the function type" to "Fix places where the return type of a FunctionDecl was being used in place of the function type". bobsayshilol added a comment. Add missing part of previous change to patch. Now all unit tests pass. I'm removing [CodeGen] from the title too since this change touches more than just that section now. https://reviews.llvm.org/D53263 Files: lib/AST/Decl.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CGNonTrivialStruct.cpp lib/CodeGen/CGObjC.cpp lib/CodeGen/CGStmtOpenMP.cpp lib/CodeGen/ItaniumCXXABI.cpp lib/Frontend/Rewrite/RewriteModernObjC.cpp Index: lib/Frontend/Rewrite/RewriteModernObjC.cpp === --- lib/Frontend/Rewrite/RewriteModernObjC.cpp +++ lib/Frontend/Rewrite/RewriteModernObjC.cpp @@ -3099,10 +3099,10 @@ SmallVectorImpl &MsgExprs, ObjCMethodDecl *Method) { // Now do the "normal" pointer to function cast. - QualType castType = getSimpleFunctionType(returnType, ArgTypes, + QualType funcType = getSimpleFunctionType(returnType, ArgTypes, Method ? Method->isVariadic() : false); - castType = Context->getPointerType(castType); + QualType castType = Context->getPointerType(funcType); // build type for containing the objc_msgSend_stret object. static unsigned stretCount=0; @@ -3177,7 +3177,7 @@ // AST for __Stretn(receiver, args).s; IdentifierInfo *ID = &Context->Idents.get(name); FunctionDecl *FD = FunctionDecl::Create(*Context, TUDecl, SourceLocation(), - SourceLocation(), ID, castType, + SourceLocation(), ID, funcType, nullptr, SC_Extern, false, false); DeclRefExpr *DRE = new (Context) DeclRefExpr(FD, false, castType, VK_RValue, SourceLocation()); Index: lib/CodeGen/ItaniumCXXABI.cpp === --- lib/CodeGen/ItaniumCXXABI.cpp +++ lib/CodeGen/ItaniumCXXABI.cpp @@ -2315,11 +2315,13 @@ FTy, GlobalInitFnName, getTypes().arrangeNullaryFunction(), SourceLocation()); ASTContext &Ctx = getContext(); +QualType ReturnTy = Ctx.VoidTy; +QualType FunctionTy = Ctx.getFunctionType(ReturnTy, llvm::None, {}); FunctionDecl *FD = FunctionDecl::Create( Ctx, Ctx.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), -&Ctx.Idents.get(GlobalInitFnName), Ctx.VoidTy, nullptr, SC_Static, +&Ctx.Idents.get(GlobalInitFnName), FunctionTy, nullptr, SC_Static, false, false); -CGF.StartFunction(GlobalDecl(FD), getContext().VoidTy, GlobalInitFn, +CGF.StartFunction(GlobalDecl(FD), ReturnTy, GlobalInitFn, getTypes().arrangeNullaryFunction(), FunctionArgList(), SourceLocation(), SourceLocation()); Index: lib/CodeGen/CGStmtOpenMP.cpp === --- lib/CodeGen/CGStmtOpenMP.cpp +++ lib/CodeGen/CGStmtOpenMP.cpp @@ -385,11 +385,11 @@ FunctionDecl *DebugFunctionDecl = nullptr; if (!FO.UIntPtrCastRequired) { FunctionProtoType::ExtProtoInfo EPI; +QualType FunctionTy = Ctx.getFunctionType(Ctx.VoidTy, llvm::None, EPI); DebugFunctionDecl = FunctionDecl::Create( Ctx, Ctx.getTranslationUnitDecl(), FO.S->getBeginLoc(), -SourceLocation(), DeclarationName(), Ctx.VoidTy, -Ctx.getTrivialTypeSourceInfo( -Ctx.getFunctionType(Ctx.VoidTy, llvm::None, EPI)), +SourceLocation(), DeclarationName(), FunctionTy, +Ctx.getTrivialTypeSourceInfo(FunctionTy), SC_Static, /*isInlineSpecified=*/false, /*hasWrittenPrototype=*/false); } for (const FieldDecl *FD : RD->fields()) { Index: lib/CodeGen/CGObjC.cpp === --- lib/CodeGen/CGObjC.cpp +++ lib/CodeGen/CGObjC.cpp @@ -3229,29 +3229,36 @@ ASTContext &C = getContext(); IdentifierInfo *II = &CGM.getContext().Idents.get("__assign_helper_atomic_property_"); + + QualType ReturnTy = C.VoidTy; + QualType DestTy = C.getPointerType(Ty); + QualType SrcTy = Ty; + SrcTy.addConst(); + SrcTy = C.getPointerType(SrcTy); + + SmallVector ArgTys; + ArgTys.push_back(DestTy); + ArgTys.push_back(SrcTy); + QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {}); + FunctionDecl *FD = FunctionDecl::Create(C, C.getTranslationUnitDecl(), SourceLocation(
[PATCH] D53329: Generate DIFile with main program if source is not available
dblaikie added a comment. @aprantl - yeah, not sure I have any big feels about this (nor do I fully understand it) @yonghong-song - could you explain this maybe in a bit more detail. What behavior does this fix provide? (compared to behavior of existing working cases that don't hit this bug, for example) Repository: rC Clang https://reviews.llvm.org/D53329 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location
vsk added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2871 + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); Why shouldn't this always be line 0? A call to a check handler is always auto-generated. Comment at: test/CodeGenCXX/ubsan-check-debuglocs.cpp:2 +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \ +// RUN: -fsanitize=null,object-size,return -fsanitize-recover=null \ +// RUN: %s -o - | FileCheck %s Are all three sanitizers needed here to reproduce the bug? Seems like a simpler test would be: ``` // RUN: ... -fsanitize=null ... int deref(int *p) { return *p; } // CHECK-LABEL: @deref // CHECK: __ubsan_handle_type_mismatch_v1{{.*}} !dbg [[ubsan_handler_loc:![0-9]+]] // CHECK: [[ubsan_handler_loc]] = !DILocation(line: 0 ``` https://reviews.llvm.org/D53459 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location
aprantl added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2871 + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); vsk wrote: > Why shouldn't this always be line 0? A call to a check handler is always > auto-generated. I was thinking that it might be nice to have the source location of the expression with the UB in the backtrace, too — not just in the error message. (Which is the current behavior by accident, since no location means carry over the previous instruction's location). But I'm fine with just setting line 0, too. Comment at: test/CodeGenCXX/ubsan-check-debuglocs.cpp:2 +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \ +// RUN: -fsanitize=null,object-size,return -fsanitize-recover=null \ +// RUN: %s -o - | FileCheck %s vsk wrote: > Are all three sanitizers needed here to reproduce the bug? Seems like a > simpler test would be: > > ``` > // RUN: ... -fsanitize=null ... > > int deref(int *p) { return *p; } > // CHECK-LABEL: @deref > // CHECK: __ubsan_handle_type_mismatch_v1{{.*}} !dbg > [[ubsan_handler_loc:![0-9]+]] > // CHECK: [[ubsan_handler_loc]] = !DILocation(line: 0 > ``` Probably not for the testcase; only to trigger the subsequent crash. https://reviews.llvm.org/D53459 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52984: [analyzer] Checker reviewer's checklist
NoQ added a comment. My take on the out-of-alpha checklist: - The checker should be evaluated on a large codebase in order to discover crashes and false positives specific to the checker. It should demonstrate the expected reasonably good results on it, with the stress on having as little false positives as possible. - The codebase should contain patterns that the checker checks. It should ideally also contain actual bugs, but it is fine if the checker was inspired by a one-of-a-kind devastating bug while there aren't many other instances of this bug around. - Warning and note messages should be clear and easy to understand, even if a bit long. - Messages should start with a capital letter (unlike Clang warnings!) and should not end with `.`. - Articles are usually omitted, eg. `Dereference of a null pointer` -> `Dereference of null pointer`. - Static Analyzer's checker API saves you from a lot of work, but it still forces you into a certain amount of boilerplate code when it comes to managing entities specific to your checker: - Almost all path-sensitive checkers need their own `BugReporterVisitor` in order to highlight interesting events along the path - or at least a call to `trackNullOrUndefValue`. //(need to wait for https://reviews.llvm.org/D52758)// - For example, SimpleStreamChecker should highlight the event of opening the file when reporting a file descriptor leak. - If the checker tracks anything in the program state, it needs to implement the `checkDeadSymbols` callback to clean the state up. - Unless there's a strong indication that the user is willing to opt-in to a stricter checking, obvious false positives that could be caused by the checker itself should be fixed. For example: - If a checker tracks state of mutable objects enumerated by symbols, escaping symbols into unfamiliar functions must invalidate the state - for example, through `checkPointerEscape`. - For example, SimpleStreamChecker should drop its knowledge about an open file descriptor when it is passed into a function that may potentially close the descriptor. At the same time, it is fine to not escape closed descriptors. - If the checker is intentionally leaving a room for false positives, i.e., it checks for a coding standard that isn't critical to follow when it comes to formal correctness of the program, then there should be a cheap explicit way to suppress the warnings when the violation of the coding standard is intentional. - Fuzzy matching of API function or type names is generally dangerous when it may lead to false positives, fine otherwise. - For example, a checker for LLVM `isa<>` API shouldn't match it as `isa*`, so that it didn't accidentally make assumptions on how `isalpha` works. - It is perfectly fine to have some fuzziness for initial experiments in alpha - that's a great way to figure out which APIs you want to cover. My take on the everyday checklist: - Checkers are encouraged to actively participate in the analysis by sharing its knowledge about the program state with the rest of the analyzer, but they should not be disrupting the analysis unnecessarily: - If a checker splits program state, this must be based on knowledge that the newly appearing branches are definitely possible and worth exploring from the user's perspective. Otherwise the state split should be delayed until there's an indication that one of the paths is taken, or one of the paths needs to be dropped entirely. - For example, it is fine to eagerly split paths while modeling `isalpha(x)` as long as `x` is constrained accordingly on each path. At the same time, it is not a good idea to split paths over the return value of `printf()` while modeling the call because nobody ever checks for errors in `printf`; at best, it'd just double the remaining analysis time. - Caution is advised when using `CheckerContext::generateNonFatalErrorNode` because it generates an independent transition, much like `addTransition`. It is easy to accidentally split paths while using it. - Ideally, try to structure the code so that it was obvious that every `addTransition` or `generateNonFatalErrorNode` (or sequence of such if the split is intended) is immediately followed by return from the checker callback. - Multiple implementations of `evalCall` in different checkers should not conflict. - When implementing `evalAssume`, the checker should always return a non-null state for either the true assumption or the false assumption (or both). - Checkers shall not mutate values of expressions, i.e. use the `ProgramState::BindExpr` API, unless they are fully responsible for computing the value. Under no circumstances should they change non-`Unknown` values of expressions. - Currently the only valid use case for this API in checkers is to model the return value in the `evalCall` callback. - If expression values are incorrect, `ExprEngine` needs to be fixed in