[cfe-dev] Patch & feedback request

Erik Verbruggen erikjv at me.com
Mon Jul 27 09:47:14 PDT 2009

Attached is a new patch with:

On Monday, 27 July, 2009, at 05:09PM, "Chris Lattner" <clattner at apple.com> wrote:
>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  
>+    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) ..
>Also, what about CV qualifiers on the interface, like this:
>@catch (const NSString *X) ..
>@catch (volatile NSString *X) ..

Does ObjC use ActOnCXXTryBlock? And as we're talking about more cases anyway, shouldn't clang warn on:

try {
} catch (Exception e) {}

... saying that an exception should be catched by reference?


-------------- next part --------------
A non-text attachment was scrubbed...
Name: unreachable-exception-handlers.patch
Type: application/octet-stream
Size: 5049 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20090727/e13209e2/attachment.obj>

More information about the cfe-dev mailing list