[cfe-commits] r167766 - in /cfe/trunk: include/clang/Sema/Scope.h lib/Parse/ParseStmt.cpp lib/Sema/IdentifierResolver.cpp test/CXX/basic/basic.scope/basic.scope.local/p2.cpp

Pawel Wodnicki pawel at 32bitmicro.com
Wed Dec 5 15:51:26 PST 2012


> 
> On Dec 5, 2012, at 3:31 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
>> On Wed, Dec 5, 2012 at 8:42 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> Richard - apparently this didn't make it into the 3.2 branch, could you approve?
>>
>> Yes, this is important. Approved, but Doug was owner here for 3.2, and I'm not sure whether Pawel is looking at the owners as listed in the branch or on trunk, so you may need his approval too.
> 
> Richard's approval of Sema patches is fine. (Approved regardless)
> 
> 

Committed revision 169451.
It is a good fix!
Thank you,
Pawel

	- Doug
> 
>> On Tue, Nov 13, 2012 at 10:53 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>> On Mon, Nov 12, 2012 at 3:41 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>> On Mon, Nov 12, 2012 at 2:25 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>> Author: dblaikie
>>>>> Date: Mon Nov 12 16:25:41 2012
>>>>> New Revision: 167766
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=167766&view=rev
>>>>> Log:
>>>>> Fix more try scoping bugs introduced by r167650.
>>>>>
>>>>> Introduces more clear scoping flags & flag combinations which should hopefully
>>>>> be more understandable.
>>>>>
>>>>> Modified:
>>>>>     cfe/trunk/include/clang/Sema/Scope.h
>>>>>     cfe/trunk/lib/Parse/ParseStmt.cpp
>>>>>     cfe/trunk/lib/Sema/IdentifierResolver.cpp
>>>>>     cfe/trunk/test/CXX/basic/basic.scope/basic.scope.local/p2.cpp
>>>>>
>>>>> Modified: cfe/trunk/include/clang/Sema/Scope.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=167766&r1=167765&r2=167766&view=diff
>>>>> ==============================================================================
>>>>> --- cfe/trunk/include/clang/Sema/Scope.h (original)
>>>>> +++ cfe/trunk/include/clang/Sema/Scope.h Mon Nov 12 16:25:41 2012
>>>>> @@ -84,11 +84,18 @@
>>>>>      /// TryScope - This is the scope of a C++ try statement.
>>>>>      TryScope = 0x1000,
>>>>>
>>>>> +    /// CatchScope - This is the scope of a C++ catch statement.
>>>>> +    CatchScope = 0x2000,
>>>>> +
>>>>> +    /// FnTryCatchScope - This is the scope for a function-level C++ try or
>>>>> +    /// catch scope.
>>>>> +    FnTryCatchScope = 0x4000,
>>>>> +
>>>>>      /// FnTryScope - This is the scope of a function-level C++ try scope.
>>>>> -    FnTryScope = 0x3000,
>>>>> +    FnTryScope = TryScope | FnTryCatchScope,
>>>>>
>>>>>      /// FnCatchScope - This is the scope of a function-level C++ catch scope.
>>>>> -    FnCatchScope = 0x4000
>>>>> +    FnCatchScope = CatchScope | FnTryCatchScope
>>>>
>>>> The other enumeration values here are all single flags, and callers |
>>>> together the relevant ones. I think the users of this class would be
>>>> clearer and more consistent without these two additional values.
>>>>
>>>> Also, you don't seem to be using the CatchScope flag for anything; is
>>>> it necessary?
>>>
>>> Addressed both of these in r167856. I think initially the grouped
>>> flags made sense when it made it easier to do x & GROUP == GROUP but
>>> after refactoring the tests in IdentifierResolver I didn't end up
>>> needing to test multiple flags simultaneously.
>>>
>>> The CatchScope flag was just for consistency but, yes, it's unused so
>>> I've removed it.
>>
> 
> 




More information about the cfe-commits mailing list