[cfe-commits] r158683 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/inline.c test/SemaCXX/inline.cpp

[Jordan Rose]

On Jun 20, 2012, at 2:23 AM, Chandler Carruth wrote:

> On Wed, Jun 20, 2012 at 12:51 AM, Chandler Carruth <chandlerc at google.com> wrote:
>> On Mon, Jun 18, 2012 at 3:09 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>> Author: jrose
>> Date: Mon Jun 18 17:09:19 2012
>> New Revision: 158683
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=158683&view=rev
>> Log:
>> Support -Winternal-linkage-in-inline in C++ code.
>> 
>> This includes treating anonymous namespaces like internal linkage, and allowing
>> const variables to be used even if internal. The whole thing's been broken out
>> into a separate function to avoid nested ifs.
>> 
> I also think this warning may be firing erroneously on some code in Clang:
> 
> ../tools/clang/lib/AST/Type.cpp:2020:50: warning: function 'getVisibility' is in an anonymous namespace but is used in an inline method with external linkage [-Winternal-linkage-in-inline]
>            T->TypeBits.getVisibility() == Result.getVisibility());
>                                                  ^
> /usr/include/assert.h:89:5: note: expanded from macro 'assert'
>   ((expr)                                                               \
>     ^
> ../tools/clang/lib/AST/Type.cpp:1964:14: note: 'getVisibility' declared here
>   Visibility getVisibility() const { return LV.visibility(); }
>              ^
> 1 warning generated.
> 
> 
> Here, 'getVisibility' is certainly in an anonymous namespace, and one might expect the function on line 2020 to have external linkage, but in fact this line is inside of a template, and that template is only ever instantiated with type parameters which have internal linkage, so I would expect all of that templates methods etc to also only have internal linkage in the final binary?

Hm. It seems like the warning is still sensible because the template is /guaranteed/ to be only instantiated here, but I see your point. Maybe this warning should fire on template instantiations instead, though that could end up getting a zillion warnings if something like SmallVector ever has this problem.

One alternative would be to say that this template has no visible definitions outside of the main file, even though it has a declaration, and so it probably won't be instantiated elsewhere. Would that be good enough?

(I feel like this is a similar issue to before, where a header file included only once doesn't happen to break ODR, but we wouldn't be able to tell from a single translation unit if that ever changed. Which led to Warning over ExtWarn.)

Thanks for catching these problems. I should really be in the habit of checking new warnings against all of Clang (now ashamed to admit I didn't for this one).