r225389 - Handle OpaqueValueExprs more intelligently in the TransformTypos tree

[Kaelyn Takata]

On Wed, Jan 14, 2015 at 7:36 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On Wed, Jan 7, 2015 at 1:16 PM, Kaelyn Takata <rikka at google.com> wrote:
>
>> Author: rikka
>> Date: Wed Jan  7 15:16:39 2015
>> New Revision: 225389
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=225389&view=rev
>> Log:
>> Handle OpaqueValueExprs more intelligently in the TransformTypos tree
>> transform.
>>
>> Also diagnose typos in the initializer of an invalid C++ declaration.
>> Both issues were hit using the same line of test code, depending on
>> whether the code was treated as C or C++.
>>
>> Fixes PR22092.
>>
>> Modified:
>>     cfe/trunk/lib/Sema/SemaDecl.cpp
>>     cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>     cfe/trunk/test/Sema/typo-correction.c
>>     cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=225389&r1=225388&r2=225389&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jan  7 15:16:39 2015
>> @@ -8574,8 +8574,10 @@ void Sema::AddInitializerToDecl(Decl *Re
>>                                  bool DirectInit, bool
>> TypeMayContainAuto) {
>>    // If there is no declaration, there was an error parsing it.  Just
>> ignore
>>    // the initializer.
>> -  if (!RealDecl || RealDecl->isInvalidDecl())
>> +  if (!RealDecl || RealDecl->isInvalidDecl()) {
>> +    CorrectDelayedTyposInExpr(Init);
>>      return;
>> +  }
>>
>>    if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(RealDecl)) {
>>      // With declarators parsed the way they are, the parser cannot
>>
>> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=225389&r1=225388&r2=225389&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jan  7 15:16:39 2015
>> @@ -6141,6 +6141,12 @@ public:
>>
>>    ExprResult TransformLambdaExpr(LambdaExpr *E) { return Owned(E); }
>>
>> +  ExprResult TransformOpaqueValueExpr(OpaqueValueExpr *E) {
>> +    if (Expr *SE = E->getSourceExpr())
>> +      return TransformExpr(SE);
>> +    return BaseTransform::TransformOpaqueValueExpr(E);
>> +  }
>>
>
> Hmm, can you instead handle this by overriding TreeTransform's
> AlreadyTransformed:
>
> bool AlreadyTransformed(QualType T) {
>   return T.isNull() || T->isInstantiationDependentType();
> }
>
> (Since we treat a TypoExpr as being dependent, it must lead to an
> instantiation-dependent type.)
>

That fixes the TreeTransform assertion reported in PR22092, but it isn't
sufficient for the TypoExpr nested inside of the OpaqueValueExpr to be seen
as the default TransformOpaqueValueExpr just returns the original
OpaqueValueExpr if the assertion doesn't fail. So instead the test case
added to typo-correction.c fails in a different way:

error: 'error' diagnostics expected but not seen:
  File ~/llvm/tools/clang/test/Sema/typo-correction.c Line 13: use of
undeclared identifier 'b'
error: 'error' diagnostics seen but not expected:
  (frontend): used type '<dependent type>' where arithmetic, pointer, or
vector type is required
clang: ../tools/clang/lib/Sema/Sema.cpp:249: clang::Sema::~Sema():
Assertion `DelayedTypos.empty() && "Uncorrected typos!"' failed.
[followed by the stack trace for the failed assertion]

Overriding the definition of AlreadyTransformed just for to get an
assertion in one Tranform*Expr method to pass also feels a bit...
heavy-handed to me.

>
>
>> +
>>    ExprResult Transform(Expr *E) {
>>      ExprResult Res;
>>      while (true) {
>>
>> Modified: cfe/trunk/test/Sema/typo-correction.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/typo-correction.c?rev=225389&r1=225388&r2=225389&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/typo-correction.c (original)
>> +++ cfe/trunk/test/Sema/typo-correction.c Wed Jan  7 15:16:39 2015
>> @@ -9,3 +9,6 @@ void PR21656() {
>>    float x;
>>    x = (float)arst;  // expected-error-re {{use of undeclared identifier
>> 'arst'{{$}}}}
>>  }
>> +
>> +a = b ? : 0;  // expected-warning {{type specifier missing, defaults to
>> 'int'}} \
>> +              // expected-error {{use of undeclared identifier 'b'}}
>>
>> Modified: cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp?rev=225389&r1=225388&r2=225389&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp Wed Jan  7
>> 15:16:39 2015
>> @@ -152,3 +152,8 @@ namespace PR21947 {
>>  int blue;  // expected-note {{'blue' declared here}}
>>  __typeof blur y;  // expected-error {{use of undeclared identifier
>> 'blur'; did you mean 'blue'?}}
>>  }
>> +
>> +namespace PR22092 {
>> +a = b ? : 0;  // expected-error {{C++ requires a type specifier for all
>> declarations}} \
>> +              // expected-error-re {{use of undeclared identifier
>> 'b'{{$}}}}
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150120/f6fb2e2f/attachment.html>