[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 9:00 , Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Jun 20, 2012, at 12:51 AM, Chandler Carruth 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.
>>> 
>> This warning seems interesting, and I generally like the correctness insistence, but the current output is really unhelpful. Let's look at one of the *many* cases I'm trying to fix in LLVM's own code:
>> 
>> In file included from ../lib/Transforms/Utils/SSAUpdater.cpp:29:
>> ../include/llvm/Transforms/Utils/SSAUpdaterImpl.h:406:54: warning: function 'PHI_begin' is in an anonymous namespace but is used in an inline method with external linkage [-Winternal-linkage-in-inline]
>>      for (typename Traits::PHI_iterator I = Traits::PHI_begin(PHI),
>>                                                     ^
>> 
>> So, for starters this message doesn't read naturally with the code snippet its pointing at because the message is a bit inverted. Imagine it instead read as:
>> 
>> "warning: calling function 'PHI_begin' from an inline method with external linkage, but this method is defined in an anonymous namespace"
>> 
>> This actually starts with the code under the cursor, and takes the user to the nature of the problem.
> 
> I see the ordering problem; there's a bit of a dangling reference here (does "this method" refer to PHI_begin or the current method?). I'll try to come up with something else.

Since the action to take will almost certainly be to change the linkage of the referenced function/variable/method, what do you think about showing the declaration site at the warning and "used here" at the note? It makes the message match the location. The downside is that the message is nowhere near the /use/, so if it's something like Xcode's warnings you'd have to jump to the note to see why this triggered.

"variable 'x' has internal linkage but is used in inline function 'y', which has external linkage"
static int x;

"used here"
++x;