On Jun 18, 2012, at 15:42 , Richard Smith <[email protected]> wrote:

> On Mon, Jun 18, 2012 at 3:28 PM, Jordan Rose <[email protected]> wrote:
>> 
>> On Jun 18, 2012, at 15:23 , Eli Friedman <[email protected]> wrote:
>> 
>>> On Mon, Jun 18, 2012 at 3:09 PM, Jordan Rose <[email protected]> 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 think it's worth pointing out that in the C++ case, the given
>>> testcase doesn't strictly violate ODR because the  definition of the
>>> function in question isn't actually used in multiple files.  Because
>>> of that, it shouldn't be an error with -pedantic-errors (the
>>> diagnostic should use Warning rather than Extension/ExtWarn), and you
>>> should watch to see if there are any bug reports with false positives.
>>> (I think false positives are unlikely, but not impossible.)
>> 
>> Ah, I see. Without cross-TU analysis, we can't tell if a function is used in 
>> multiple files or not. I think it's valid to leave this as ExtWarn when it's 
>> in a header file…it's kind of a ticking time bomb.
> 
> -pedantic-errors shouldn't cause us to reject valid code, and not all
> #included files are intended to be included multiple times (we have
> several examples of this in Clang, with (for instance) files generated
> by tablegen).
> 
> Have you considered implementing the check for whether a variable used
> within such an inline function has a literal type (or, in C++98, an
> integral or enumeration type), and checking whether the initializer is
> a constant expression?
> 
> Checking whether the variable's address is used seems trickier,
> perhaps we can use the result of the odr-use checking mechanism?

I skipped the literal type check; currently it allows all const variables with 
initializers, literal or not. There's also no check for the address clause 
(which I mistakenly interpreted as equivalent to "prvalue only", but which 
allows references). Those checks won't cause us to reject valid code.

The only valid code we will reject under -pedantic-errors here is code that 
references internal linkage / anonymous namespace non-constant variables or 
functions from a file that is included in one translation unit in the entire 
compilation. From the perspective of perfection, this is technically incorrect, 
and for -pedantic-errors that may be what we want. But there is never a case 
where this is not fixable (by putting the offending function/method in an 
anonymous namespace), and if someday the included file shows up in two 
translation units you have a legitimate pedantic-error which could affect the 
behavior of your program.

I'm genuinely not sure which is worse: missing an error because we don't do 
cross-TU checking, or preventing compilation on the case where something is 
only included once. We'd warn whether it's ExtWarn or Warning, so that's not 
the issue.


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to