On Jun 18, 2012, at 16:21 , Richard Smith <[email protected]> wrote:
> On Mon, Jun 18, 2012 at 4:06 PM, Jordan Rose <[email protected]> wrote: >> >> 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. > > Changing ExtWarn -> Warning seems to have no downside. It just makes > -pedantic-errors more pedantically correct, which is, after all, the > point. If someone is using -pedantic-errors but not -Werror, they will > get what they asked for -- allowing this code is not an extension, > after all. Okay, got it. Thanks, Richard and Eli. Will change. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
