r225389 - Handle OpaqueValueExprs more intelligently in the TransformTypos tree
Richard Smith
richard at metafoo.co.uk
Tue Jan 20 15:26:10 PST 2015
On Tue, Jan 20, 2015 at 1:28 PM, Kaelyn Takata <rikka at google.com> wrote:
>
>
> 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;
>>> + }
>>>:61
>>> 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.
The idea is that the OpaqueValueExpr is "owned" by some other AST
node, and transforming that other AST node should cause the source
expr of the OVE to be transformed and should recreate the OVE itself.
So the TreeTransform should *never* see an OVE that needs
transformation (actually, it should probably never see one at all...).
In any case, it can't know how to transform one, because that's a
non-local property and depends on the owner of the OVE.
For a BinaryConditionalOperator, we transform the "common" expr (not
an OVE) and the RHS expr (also not an OVE). I ran your patch against
this testcase:
int foobar; int f() { return goobar ?: 1; }
... and your TransformOpaqueValueExpr function was not called. So I
think that part is entirely a red herring.
In your testing, did you revert just the TransformOpaqueValueExpr part
of this patch or also the change to SemaDecl (which I think was the
real fix here)?
>>> +
>>> 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
>>
>>
>
More information about the cfe-commits
mailing list