[cfe-commits] r121720 - in /cfe/trunk: include/clang/AST/TypeLoc.h lib/AST/TypeLoc.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp

[John McCall]

On Dec 13, 2010, at 11:43 PM, Enea Zaffanella wrote:

> Il 14/12/2010 00:06, John McCall ha scritto:
>> On Dec 13, 2010, at 2:40 PM, Chandler Carruth wrote:
>>> On Mon, Dec 13, 2010 at 2:27 PM, Abramo Bagnara
>>> <abramo.bagnara at gmail.com <mailto:abramo.bagnara at gmail.com>> wrote:
>>> 
>>>    Skip ParenType on function instantiations.
>>> 
>>> 
>>> While this fixes one case, your patch remains fundamentally broken.
>>> There are more cases throughout Sema that reason about a QualType or
>>> TypeSourceInfo or TypeLoc and expect getAs<FunctionType> or
>>> dyn_cast<FunctionType> to Just Work. They don't any more. Function
>>> attributes won't work now for example. Can you audit the codebase
>>> looking for these? They should be very easy to craft test cases for.
>> 
>> getAs<>() should be working just fine because (1) the canonical type is
>> untouched and (2) isSugared() and desugar() have been implemented
>> correctly.
> 
> Yes, we were relying on this behaviour in many places.
> 
>> Chandler is right that we probably need to check all the
>> explicit isa<>s on TypeLocs.
> 
> OK, we'll perform such a review.
> 
>> I am skeptical that we need QualType::IgnoreParens() at all,
> 
> Currently we only have a couple of uses in the very same function and
> you are right, these are unnecessary, since in those places too we have
> occurrences of getAs<>, which are going to work just fine. We will
> remove this method.
> 
>> but TypeLoc::IgnoreParens is reasonable, although I agree with Chandler that
>> this should be an inline function.
> 
> Agreed: we will code the fast path inline and use an IgnoreParensImpl
> for the slow path.

I think there's not much harm in leaving it all inline, but however you think best.
If you're going to make it out-of-line, though, please make it a static member
passed a TypeLoc by value.

John.