r206202 - Add clang:: qualifier back to Module

[Nick Lewycky]

On 14 April 2014 11:59, Ben Langmuir <blangmuir at apple.com> wrote:

>
> On Apr 14, 2014, at 11:53 AM, Nick Lewycky <nlewycky at google.com> wrote:
>
> On 14 April 2014 11:21, Ben Langmuir <blangmuir at apple.com> wrote:
>
>> Author: benlangmuir
>> Date: Mon Apr 14 13:21:04 2014
>> New Revision: 206202
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=206202&view=rev
>> Log:
>> Add clang:: qualifier back to Module
>>
>> Apparently this was required by some compilers.
>>
>
> I don't believe this fix is correct, I think the code is still invalid
> even if it happens to compile, since it's still changing the meaning of the
> name "Module" inside a class scope.
>
>
> Fair enough. I’ll take a look at renaming this later if no one beats me to
> it.  I just wanted to make sure it built for the time being.
>

Richard pointed out to me offline that your fix is indeed correct, but it's
not entirely clear me to me how fragile that fact is. In particular, it's
okay to create such a conflict (where the name means two different things)
as long as no name lookup is performed on that name. After the class is
complete (before delayed parsing), the name lookup on "Module" will simply
find the variable and not the type.

So this is okay:
struct X { clang::Module *Module; };

and so is this, in the sense that it's a clear error both clang and gcc
diagnose:
struct X { clang::Module *Module; Module *M; }; // second Module is a use
of the variable name, never the type

Nick

> The compilers which issued these errors were certainly correct, I've filed
> PR19427 for clang to detect this some day.
>
> Nick
>
>
>> Modified:
>>     cfe/trunk/include/clang/Frontend/FrontendActions.h
>>
>> Modified: cfe/trunk/include/clang/Frontend/FrontendActions.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendActions.h?rev=206202&r1=206201&r2=206202&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Frontend/FrontendActions.h (original)
>> +++ cfe/trunk/include/clang/Frontend/FrontendActions.h Mon Apr 14
>> 13:21:04 2014
>> @@ -93,7 +93,7 @@ public:
>>  };
>>
>>  class GenerateModuleAction : public ASTFrontendAction {
>> -  Module *Module;
>> +  clang::Module *Module;
>>    const FileEntry *ModuleMapForUniquing;
>>    bool IsSystem;
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140414/fedc5ff7/attachment.html>