Hi, I think this test is still broken on Windows when you are building a cross compiler as we at Sony do. I have put the details in PR40033. Can somebody take a look?
Douglas Yung From: cfe-commits <cfe-commits-boun...@lists.llvm.org> On Behalf Of Reid Kleckner via cfe-commits Sent: Thursday, December 13, 2018 13:51 To: Ilya Biryukov <ibiryu...@google.com> Cc: cfe-commits <cfe-commits@lists.llvm.org> Subject: Re: r349053 - [CodeComplete] Fill preferred type on binary expressions r349086 should take care of it, but you may want to tweak it. On Thu, Dec 13, 2018 at 1:30 PM Reid Kleckner <r...@google.com<mailto:r...@google.com>> wrote: This new test doesn't pass on Windows. I think it's an LLP64-ness bug based on the output: Note: Google Test filter = PreferredTypeTest.Binar* [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from PreferredTypeTest [ RUN ] PreferredTypeTest.BinaryExpr test.cc:4:45: error: invalid operands to binary expression ('float' and 'int') x += 10; x -= 10; x *= 10; x /= 10; x %= 10; ~ ^ ~~ 1 error generated. test.cc:4:45: error: invalid operands to binary expression ('float' and 'int') x += 10; x -= 10; x *= 10; x /= 10; x %= 10; ~ ^ ~~ 1 error generated. test.cc:4:45: error: invalid operands to binary expression ('float' and 'int') x += 10; x -= 10; x *= 10; x /= 10; x %= 10; ~ ^ ~~ 1 error generated. test.cc:4:45: error: invalid operands to binary expression ('float' and 'int') x += 10; x -= 10; x *= 10; x /= 10; x %= 10; ~ ^ ~~ 1 error generated. test.cc:4:45: error: invalid operands to binary expression ('float' and 'int') x += 10; x -= 10; x *= 10; x /= 10; x %= 10; ~ ^ ~~ 1 error generated. C:\src\llvm-project\clang\unittests\Sema\CodeCompleteTest.cpp(216): error: Value of: collectPreferredTypes(Code) Expected: only contains elements that is equal to "long" Actual: { "long long", "long long", "long long" }, whose element #0 doesn't match On Thu, Dec 13, 2018 at 8:09 AM Ilya Biryukov via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: Author: ibiryukov Date: Thu Dec 13 08:06:11 2018 New Revision: 349053 URL: http://llvm.org/viewvc/llvm-project?rev=349053&view=rev Log: [CodeComplete] Fill preferred type on binary expressions Reviewers: kadircet Reviewed By: kadircet Subscribers: arphaman, cfe-commits Differential Revision: https://reviews.llvm.org/D55648 Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Parse/ParseExpr.cpp cfe/trunk/lib/Sema/SemaCodeComplete.cpp cfe/trunk/test/Index/complete-exprs.c cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=349053&r1=349052&r2=349053&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Thu Dec 13 08:06:11 2018 @@ -10350,7 +10350,7 @@ public: void CodeCompleteInitializer(Scope *S, Decl *D); void CodeCompleteReturn(Scope *S); void CodeCompleteAfterIf(Scope *S); - void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS); + void CodeCompleteBinaryRHS(Scope *S, Expr *LHS, tok::TokenKind Op); void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS, bool EnteringContext, QualType BaseType); Modified: cfe/trunk/lib/Parse/ParseExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=349053&r1=349052&r2=349053&view=diff ============================================================================== --- cfe/trunk/lib/Parse/ParseExpr.cpp (original) +++ cfe/trunk/lib/Parse/ParseExpr.cpp Thu Dec 13 08:06:11 2018 @@ -393,10 +393,11 @@ Parser::ParseRHSOfBinaryExpression(ExprR } } - // Code completion for the right-hand side of an assignment expression - // goes through a special hook that takes the left-hand side into account. - if (Tok.is(tok::code_completion) && NextTokPrec == prec::Assignment) { - Actions.CodeCompleteAssignmentRHS(getCurScope(), LHS.get()); + // Code completion for the right-hand side of a binary expression goes + // through a special hook that takes the left-hand side into account. + if (Tok.is(tok::code_completion)) { + Actions.CodeCompleteBinaryRHS(getCurScope(), LHS.get(), + OpToken.getKind()); cutOffParsing(); return ExprError(); } Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=349053&r1=349052&r2=349053&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original) +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Dec 13 08:06:11 2018 @@ -4878,9 +4878,86 @@ void Sema::CodeCompleteAfterIf(Scope *S) Results.data(), Results.size()); } -void Sema::CodeCompleteAssignmentRHS(Scope *S, Expr *LHS) { - if (LHS) - CodeCompleteExpression(S, static_cast<Expr *>(LHS)->getType()); +static QualType getPreferredTypeOfBinaryRHS(Sema &S, Expr *LHS, + tok::TokenKind Op) { + if (!LHS) + return QualType(); + + QualType LHSType = LHS->getType(); + if (LHSType->isPointerType()) { + if (Op == tok::plus || Op == tok::plusequal || Op == tok::minusequal) + return S.getASTContext().getPointerDiffType(); + // Pointer difference is more common than subtracting an int from a pointer. + if (Op == tok::minus) + return LHSType; + } + + switch (Op) { + // No way to infer the type of RHS from LHS. + case tok::comma: + return QualType(); + // Prefer the type of the left operand for all of these. + // Arithmetic operations. + case tok::plus: + case tok::plusequal: + case tok::minus: + case tok::minusequal: + case tok::percent: + case tok::percentequal: + case tok::slash: + case tok::slashequal: + case tok::star: + case tok::starequal: + // Assignment. + case tok::equal: + // Comparison operators. + case tok::equalequal: + case tok::exclaimequal: + case tok::less: + case tok::lessequal: + case tok::greater: + case tok::greaterequal: + case tok::spaceship: + return LHS->getType(); + // Binary shifts are often overloaded, so don't try to guess those. + case tok::greatergreater: + case tok::greatergreaterequal: + case tok::lessless: + case tok::lesslessequal: + if (LHSType->isIntegralOrEnumerationType()) + return S.getASTContext().IntTy; + return QualType(); + // Logical operators, assume we want bool. + case tok::ampamp: + case tok::pipepipe: + case tok::caretcaret: + return S.getASTContext().BoolTy; + // Operators often used for bit manipulation are typically used with the type + // of the left argument. + case tok::pipe: + case tok::pipeequal: + case tok::caret: + case tok::caretequal: + case tok::amp: + case tok::ampequal: + if (LHSType->isIntegralOrEnumerationType()) + return LHSType; + return QualType(); + // RHS should be a pointer to a member of the 'LHS' type, but we can't give + // any particular type here. + case tok::periodstar: + case tok::arrowstar: + return QualType(); + default: + assert(false && "unhandled binary op"); + return QualType(); + } +} + +void Sema::CodeCompleteBinaryRHS(Scope *S, Expr *LHS, tok::TokenKind Op) { + auto PreferredType = getPreferredTypeOfBinaryRHS(*this, LHS, Op); + if (!PreferredType.isNull()) + CodeCompleteExpression(S, PreferredType); else CodeCompleteOrdinaryName(S, PCC_Expression); } Modified: cfe/trunk/test/Index/complete-exprs.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-exprs.c?rev=349053&r1=349052&r2=349053&view=diff ============================================================================== --- cfe/trunk/test/Index/complete-exprs.c (original) +++ cfe/trunk/test/Index/complete-exprs.c Thu Dec 13 08:06:11 2018 @@ -33,16 +33,11 @@ void f5(float f) { // CHECK-CC1: ParmDecl:{ResultType int}{TypedText j} (8) // CHECK-CC1: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen (}{Placeholder expression-or-type}{RightParen )} (40) // RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:10 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s -// RUN: c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s -// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s -// CHECK-CC3: macro definition:{TypedText __VERSION__} (70) -// CHECK-CC3: FunctionDecl:{ResultType int}{TypedText f}{LeftParen (}{Placeholder int}{RightParen )} (50) -// CHECK-CC3-NOT: NotImplemented:{TypedText float} -// CHECK-CC3: ParmDecl:{ResultType int}{TypedText j} (34) -// CHECK-CC3: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen (}{Placeholder expressio +// RUN: c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s +// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s -// RUN: c-index-test -code-completion-at=%s:7:18 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s -// RUN: c-index-test -code-completion-at=%s:7:22 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s +// RUN: c-index-test -code-completion-at=%s:7:18 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s +// RUN: c-index-test -code-completion-at=%s:7:22 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s // RUN: c-index-test -code-completion-at=%s:7:2 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC2 %s // CHECK-CC2: macro definition:{TypedText __VERSION__} (70) // CHECK-CC2: FunctionDecl:{ResultType int}{TypedText f}{LeftParen (}{Placeholder int}{RightParen )} (50) Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=349053&r1=349052&r2=349053&view=diff ============================================================================== --- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original) +++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Dec 13 08:06:11 2018 @@ -14,31 +14,39 @@ #include "clang/Sema/Sema.h" #include "clang/Sema/SemaDiagnostic.h" #include "clang/Tooling/Tooling.h" -#include "gtest/gtest.h" #include "gmock/gmock.h" +#include "gtest/gtest.h" +#include <cstddef> +#include <string> namespace { using namespace clang; using namespace clang::tooling; +using ::testing::Each; using ::testing::UnorderedElementsAre; const char TestCCName[] = "test.cc"; -using VisitedContextResults = std::vector<std::string>; -class VisitedContextFinder: public CodeCompleteConsumer { +struct CompletionContext { + std::vector<std::string> VisitedNamespaces; + std::string PreferredType; +}; + +class VisitedContextFinder : public CodeCompleteConsumer { public: - VisitedContextFinder(VisitedContextResults &Results) + VisitedContextFinder(CompletionContext &ResultCtx) : CodeCompleteConsumer(/*CodeCompleteOpts=*/{}, /*CodeCompleteConsumer*/ false), - VCResults(Results), + ResultCtx(ResultCtx), CCTUInfo(std::make_shared<GlobalCodeCompletionAllocator>()) {} void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context, CodeCompletionResult *Results, unsigned NumResults) override { - VisitedContexts = Context.getVisitedContexts(); - VCResults = getVisitedNamespace(); + ResultCtx.VisitedNamespaces = + getVisitedNamespace(Context.getVisitedContexts()); + ResultCtx.PreferredType = Context.getPreferredType().getAsString(); } CodeCompletionAllocator &getAllocator() override { @@ -47,7 +55,9 @@ public: CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; } - std::vector<std::string> getVisitedNamespace() const { +private: + std::vector<std::string> getVisitedNamespace( + CodeCompletionContext::VisitedContextSet VisitedContexts) const { std::vector<std::string> NSNames; for (const auto *Context : VisitedContexts) if (const auto *NS = llvm::dyn_cast<NamespaceDecl>(Context)) @@ -55,27 +65,25 @@ public: return NSNames; } -private: - VisitedContextResults& VCResults; + CompletionContext &ResultCtx; CodeCompletionTUInfo CCTUInfo; - CodeCompletionContext::VisitedContextSet VisitedContexts; }; class CodeCompleteAction : public SyntaxOnlyAction { public: - CodeCompleteAction(ParsedSourceLocation P, VisitedContextResults &Results) - : CompletePosition(std::move(P)), VCResults(Results) {} + CodeCompleteAction(ParsedSourceLocation P, CompletionContext &ResultCtx) + : CompletePosition(std::move(P)), ResultCtx(ResultCtx) {} bool BeginInvocation(CompilerInstance &CI) override { CI.getFrontendOpts().CodeCompletionAt = CompletePosition; - CI.setCodeCompletionConsumer(new VisitedContextFinder(VCResults)); + CI.setCodeCompletionConsumer(new VisitedContextFinder(ResultCtx)); return true; } private: // 1-based code complete position <Line, Col>; ParsedSourceLocation CompletePosition; - VisitedContextResults& VCResults; + CompletionContext &ResultCtx; }; ParsedSourceLocation offsetToPosition(llvm::StringRef Code, size_t Offset) { @@ -88,21 +96,49 @@ ParsedSourceLocation offsetToPosition(ll static_cast<unsigned>(Offset - StartOfLine + 1)}; } -VisitedContextResults runCodeCompleteOnCode(StringRef Code) { - VisitedContextResults Results; - auto TokenOffset = Code.find('^'); - assert(TokenOffset != StringRef::npos && - "Completion token ^ wasn't found in Code."); - std::string WithoutToken = Code.take_front(TokenOffset); - WithoutToken += Code.drop_front(WithoutToken.size() + 1); - assert(StringRef(WithoutToken).find('^') == StringRef::npos && - "expected exactly one completion token ^ inside the code"); - +CompletionContext runCompletion(StringRef Code, size_t Offset) { + CompletionContext ResultCtx; auto Action = llvm::make_unique<CodeCompleteAction>( - offsetToPosition(WithoutToken, TokenOffset), Results); + offsetToPosition(Code, Offset), ResultCtx); clang::tooling::runToolOnCodeWithArgs(Action.release(), Code, {"-std=c++11"}, TestCCName); - return Results; + return ResultCtx; +} + +struct ParsedAnnotations { + std::vector<size_t> Points; + std::string Code; +}; + +ParsedAnnotations parseAnnotations(StringRef AnnotatedCode) { + ParsedAnnotations R; + while (!AnnotatedCode.empty()) { + size_t NextPoint = AnnotatedCode.find('^'); + if (NextPoint == StringRef::npos) { + R.Code += AnnotatedCode; + AnnotatedCode = ""; + break; + } + R.Code += AnnotatedCode.substr(0, NextPoint); + R.Points.push_back(R.Code.size()); + + AnnotatedCode = AnnotatedCode.substr(NextPoint + 1); + } + return R; +} + +CompletionContext runCodeCompleteOnCode(StringRef AnnotatedCode) { + ParsedAnnotations P = parseAnnotations(AnnotatedCode); + assert(P.Points.size() == 1 && "expected exactly one annotation point"); + return runCompletion(P.Code, P.Points.front()); +} + +std::vector<std::string> collectPreferredTypes(StringRef AnnotatedCode) { + ParsedAnnotations P = parseAnnotations(AnnotatedCode); + std::vector<std::string> Types; + for (size_t Point : P.Points) + Types.push_back(runCompletion(P.Code, Point).PreferredType); + return Types; } TEST(SemaCodeCompleteTest, VisitedNSForValidQualifiedId) { @@ -119,7 +155,8 @@ TEST(SemaCodeCompleteTest, VisitedNSForV inline namespace bar { using namespace ns3::nns3; } } // foo namespace ns { foo::^ } - )cpp"); + )cpp") + .VisitedNamespaces; EXPECT_THAT(VisitedNS, UnorderedElementsAre("foo", "ns1", "ns2", "ns3::nns3", "foo::(anonymous)")); } @@ -127,7 +164,8 @@ TEST(SemaCodeCompleteTest, VisitedNSForV TEST(SemaCodeCompleteTest, VisitedNSForInvalideQualifiedId) { auto VisitedNS = runCodeCompleteOnCode(R"cpp( namespace ns { foo::^ } - )cpp"); + )cpp") + .VisitedNamespaces; EXPECT_TRUE(VisitedNS.empty()); } @@ -138,8 +176,150 @@ TEST(SemaCodeCompleteTest, VisitedNSWith void f(^) {} } } - )cpp"); + )cpp") + .VisitedNamespaces; EXPECT_THAT(VisitedNS, UnorderedElementsAre("n1", "n1::n2")); } +TEST(PreferredTypeTest, BinaryExpr) { + // Check various operations for arithmetic types. + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int x) { + x = ^10; + x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10; + x + ^10; x - ^10; x * ^10; x / ^10; x % ^10; + })cpp"), + Each("int")); + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(float x) { + x = ^10; + x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10; + x + ^10; x - ^10; x * ^10; x / ^10; x % ^10; + })cpp"), + Each("float")); + + // Pointer types. + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int *ptr) { + ptr - ^ptr; + ptr = ^ptr; + })cpp"), + Each("int *")); + + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int *ptr) { + ptr + ^10; + ptr += ^10; + ptr -= ^10; + })cpp"), + Each("long")); // long is normalized 'ptrdiff_t'. + + // Comparison operators. + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int i) { + i <= ^1; i < ^1; i >= ^1; i > ^1; i == ^1; i != ^1; + } + )cpp"), + Each("int")); + + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int *ptr) { + ptr <= ^ptr; ptr < ^ptr; ptr >= ^ptr; ptr > ^ptr; + ptr == ^ptr; ptr != ^ptr; + } + )cpp"), + Each("int *")); + + // Relational operations. + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int i, int *ptr) { + i && ^1; i || ^1; + ptr && ^1; ptr || ^1; + } + )cpp"), + Each("_Bool")); + + // Bitwise operations. + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(long long ll) { + ll | ^1; ll & ^1; + } + )cpp"), + Each("long long")); + + EXPECT_THAT(collectPreferredTypes(R"cpp( + enum A {}; + void test(A a) { + a | ^1; a & ^1; + } + )cpp"), + Each("enum A")); + + EXPECT_THAT(collectPreferredTypes(R"cpp( + enum class A {}; + void test(A a) { + // This is technically illegal with the 'enum class' without overloaded + // operators, but we pretend it's fine. + a | ^a; a & ^a; + } + )cpp"), + Each("enum A")); + + // Binary shifts. + EXPECT_THAT(collectPreferredTypes(R"cpp( + void test(int i, long long ll) { + i << ^1; ll << ^1; + i <<= ^1; i <<= ^1; + i >> ^1; ll >> ^1; + i >>= ^1; i >>= ^1; + } + )cpp"), + Each("int")); + + // Comma does not provide any useful information. + EXPECT_THAT(collectPreferredTypes(R"cpp( + class Cls {}; + void test(int i, int* ptr, Cls x) { + (i, ^i); + (ptr, ^ptr); + (x, ^x); + } + )cpp"), + Each("NULL TYPE")); + + // User-defined types do not take operator overloading into account. + // However, they provide heuristics for some common cases. + EXPECT_THAT(collectPreferredTypes(R"cpp( + class Cls {}; + void test(Cls c) { + // we assume arithmetic and comparions ops take the same type. + c + ^c; c - ^c; c * ^c; c / ^c; c % ^c; + c == ^c; c != ^c; c < ^c; c <= ^c; c > ^c; c >= ^c; + // same for the assignments. + c = ^c; c += ^c; c -= ^c; c *= ^c; c /= ^c; c %= ^c; + } + )cpp"), + Each("class Cls")); + + EXPECT_THAT(collectPreferredTypes(R"cpp( + class Cls {}; + void test(Cls c) { + // we assume relational ops operate on bools. + c && ^c; c || ^c; + } + )cpp"), + Each("_Bool")); + + EXPECT_THAT(collectPreferredTypes(R"cpp( + class Cls {}; + void test(Cls c) { + // we make no assumptions about the following operators, since they are + // often overloaded with a non-standard meaning. + c << ^c; c >> ^c; c | ^c; c & ^c; + c <<= ^c; c >>= ^c; c |= ^c; c &= ^c; + } + )cpp"), + Each("NULL TYPE")); +} + } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
