r202167 - Pretty Printer: Fix printing of conversion operator decls and calls.

[Benjamin Kramer]

On Tue, Feb 25, 2014 at 8:58 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Tue, Feb 25, 2014 at 9:26 AM, Benjamin Kramer <benny.kra at googlemail.com>
> wrote:
>>
>> Author: d0k
>> Date: Tue Feb 25 11:26:26 2014
>> New Revision: 202167
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=202167&view=rev
>> Log:
>> Pretty Printer: Fix printing of conversion operator decls and calls.
>>
>> - Don't emit anything when we encounter a call to a conversion operator.
>>     "bar(a & b)" instead of "bar(a & b.operator int())"
>>   This preserves the semantics and is still idempotent if we print the AST
>> multiple times.
>>
>> - Properly print declarations of conversion operators.
>>     "explicit operator bool();" instead of "bool operator _Bool();"
>>
>> PR18776.
>>
>> Modified:
>>     cfe/trunk/lib/AST/DeclPrinter.cpp
>>     cfe/trunk/lib/AST/DeclarationName.cpp
>>     cfe/trunk/lib/AST/StmtPrinter.cpp
>>     cfe/trunk/test/SemaCXX/ast-print.cpp
>>
>> Modified: cfe/trunk/lib/AST/DeclPrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclPrinter.cpp?rev=202167&r1=202166&r2=202167&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclPrinter.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclPrinter.cpp Tue Feb 25 11:26:26 2014
>> @@ -385,6 +385,7 @@ void DeclPrinter::VisitEnumConstantDecl(
>>
>>  void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
>>    CXXConstructorDecl *CDecl = dyn_cast<CXXConstructorDecl>(D);
>> +  CXXConversionDecl *ConversionDecl = dyn_cast<CXXConversionDecl>(D);
>>    if (!Policy.SuppressSpecifiers) {
>>      switch (D->getStorageClass()) {
>>      case SC_None: break;
>> @@ -398,7 +399,8 @@ void DeclPrinter::VisitFunctionDecl(Func
>>      if (D->isInlineSpecified())  Out << "inline ";
>>      if (D->isVirtualAsWritten()) Out << "virtual ";
>>      if (D->isModulePrivate())    Out << "__module_private__ ";
>> -    if (CDecl && CDecl->isExplicitSpecified())
>> +    if ((CDecl && CDecl->isExplicitSpecified()) ||
>> +        (ConversionDecl && ConversionDecl->isExplicit()))
>>        Out << "explicit ";
>>    }
>>
>> @@ -536,15 +538,15 @@ void DeclPrinter::VisitFunctionDecl(Func
>>          }
>>          Out << ")";
>>        }
>> -      if (!Proto.empty())
>> -        Out << Proto;
>> -    } else {
>> +    } else if (!ConversionDecl) {
>>        if (FT && FT->hasTrailingReturn()) {
>>          Out << "auto " << Proto << " -> ";
>>          Proto.clear();
>>        }
>>        AFT->getReturnType().print(Out, Policy, Proto);
>> +      Proto.clear();
>>      }
>> +    Out << Proto;
>>    } else {
>>      Ty.print(Out, Policy, Proto);
>>    }
>>
>> Modified: cfe/trunk/lib/AST/DeclarationName.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=202167&r1=202166&r2=202167&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclarationName.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclarationName.cpp Tue Feb 25 11:26:26 2014
>> @@ -191,6 +191,7 @@ raw_ostream &operator<<(raw_ostream &OS,
>>        return OS << *Rec->getDecl();
>>      LangOptions LO;
>>      LO.CPlusPlus = true;
>> +    LO.Bool = true;
>>      return OS << Type.getAsString(PrintingPolicy(LO));
>>    }
>>    case DeclarationName::CXXUsingDirective:
>> @@ -546,6 +547,7 @@ void DeclarationNameInfo::printName(raw_
>>          OS << "operator ";
>>        LangOptions LO;
>>        LO.CPlusPlus = true;
>> +      LO.Bool = true;
>>        OS << TInfo->getType().getAsString(PrintingPolicy(LO));
>>      } else
>>        OS << Name;
>>
>> Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=202167&r1=202166&r2=202167&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
>> +++ cfe/trunk/lib/AST/StmtPrinter.cpp Tue Feb 25 11:26:26 2014
>> @@ -1296,6 +1296,12 @@ void StmtPrinter::VisitCXXOperatorCallEx
>>  }
>>
>>  void StmtPrinter::VisitCXXMemberCallExpr(CXXMemberCallExpr *Node) {
>> +  // If we have a conversion operator call only print the argument.
>> +  CXXMethodDecl *MD = Node->getMethodDecl();
>> +  if (MD && isa<CXXConversionDecl>(MD)) {
>> +    PrintExpr(Node->getImplicitObjectArgument());
>> +    return;
>> +  }
>
>
> You should only do this if we have an implicit call to a conversion
> operator. Given:
>
>   auto x = (a & b).operator void*();
>
> ... we should print out the call to 'operator void*'. I think the way to
> detect this in the current AST is to look for a CastExpr whose cast kind is
> CK_UserDefinedConversion, and to skip the CXXMemberCallExpr or
> CXXConstructExpr inside it -- but maybe we should change the AST
> representation here to more directly distinguish between an implicit
> conversion operator call and an explicit one?

Using the CastExpr won't work for my use case. I have a
CXXMemberCallExpr and I want to print it. The result used to be:

Input: a & b
Output: a & b.operator void*()

so the StmtPrinter has changed semantics. The options I had was either
add parens unconditionally. Then we lose idempotence as all parens
already there will also be added, leading to gradual LISPification. Or
just drop the explicit call.

I think a flag on the expression to see whether the call was written
out explicitly would work, but I'm not sure how to implement it
properly.

- Ben

>
>>
>>    VisitCallExpr(cast<CallExpr>(Node));
>>  }
>>
>>
>> Modified: cfe/trunk/test/SemaCXX/ast-print.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print.cpp?rev=202167&r1=202166&r2=202167&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/ast-print.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/ast-print.cpp Tue Feb 25 11:26:26 2014
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -ast-print %s | FileCheck %s
>> +// RUN: %clang_cc1 -ast-print %s -std=gnu++11 | FileCheck %s
>>
>>  // CHECK: r;
>>  // CHECK-NEXT: (r->method());
>> @@ -173,3 +173,26 @@ void test14() {
>>  float test15() {
>>    return __builtin_asinf(1.0F);
>>  }
>> +
>> +namespace PR18776 {
>> +struct A {
>> +  operator void *();
>> +  explicit operator bool();
>> +  A operator&(A);
>> +};
>> +
>> +// CHECK: struct A
>> +// CHECK-NEXT: {{^[ ]*operator}} void *();
>> +// CHECK-NEXT: {{^[ ]*explicit}} operator bool();
>> +
>> +void bar(void *);
>> +
>> +void foo() {
>> +  A a, b;
>> +  bar(a & b);
>> +// CHECK: bar(a & b);
>> +  if (a & b)
>> +// CHECK: if (a & b)
>> +    return;
>> +}
>> +};
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>