[cfe-dev] Patch & feedback request

[Jean-Daniel Dupas]

Le 27 juil. 09 à 17:09, Chris Lattner a écrit :

>
> On Jul 27, 2009, at 6:14 AM, Erik Verbruggen wrote:
>
>>
>> On Monday, 27 July, 2009, at 01:16PM, "Sebastian Redl" <sebastian.redl at getdesigned.at
>>> wrote:
>>
>>> I believe, from previous reviews I've seen, that an operator <
>>> should only
>>> be defined if there is a natural ordering. If you simply need an
>>> ordering
>>> for a std::map (which you may want to replace by one of LLVM's own
>>> ADTs, by
>>> the way - another point that comes up often, though it depends on
>>> what you
>>> do with the map), it is, I believe, generally preferred to write a
>>> custom
>>> ordering predicate and supply it as a template parameter.
>>
>> A new patch is attached, which:
>> - has a custom ordering predecate for QualType
>> - fixes a range bug (where the last handler wasn't checked)
>> - fixes a crash when the caught exception type is null (i.e. for
>> catch(...))
>> - adds a testcase
>>
>> On the map type: I checked http://llvm.org/docs/ProgrammersManual.html#ds_map
>> and none of the other datatypes seem applicable.
>
> Instead of using std::map, please use a SmallVector<pair<>, 8> and
> then use array_pod_sort (from llvm/ADT/STLExtras.h) to sort it after
> it is populated.  A simple linear scan can then detect dupes.
>
> It looks like some tabs are sneaking in:
>
> +    // Detect handlers for the same type as an earlier one.
> +    const QualType QT = Handler->getCaughtType();
> +		if (QT.isNull())
> +			continue;
>
>
> You shouldn't use getCanonicalTypeInternal here, you should use
> ASTContext::getCanonicalType:
>
> +    const QualType CQT = QT.getTypePtr()->getCanonicalTypeInternal();
>
> How does objc treat CV qualifiers on catch types?  I don't think that
> this makes sense, does it?
>
> @catch (NSString *const X) ..
> @catch (NSString *volatile X) ..
>

Just a side note, even @catch(NSString *) does not really make sense  
in fact, at least in 64-bit, as the frameworks require that you do not  
throw something that does not inherits NSException.

 From http://developer.apple.com/releasenotes/Cocoa/RN-ObjectiveC/

The Cocoa frameworks require that all exceptions be instances of  
NSException or its subclasses. Do not throw objects of other types.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20090727/8aa3bf07/attachment.html>