[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]

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.