[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

[David Blaikie]

Richard - apparently this didn't make it into the 3.2 branch, could you approve?

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.