r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325

[Chandler Carruth]

On Thu, Jan 9, 2014 at 3:44 AM, Alp Toker <alp at nuanti.com> wrote:

> Reserved names are reserved for any use. That can include an
>> implementation specifically blessing the introduction of a name that uses
>> double underscores.
>>
>
> That's a pretty abstract argument and the standard says nothing like that.
>
> Regardless, the implementation I'm using here doesn't have such a
> provision.


I don't know what implementation you're referring to or why this is a
problem. This works with Clang and GCC. I've not seen any Windows builders
break because of this either, but maybe I missed it. I'm moderately
confident this works with ICC and xlC as well.


>
>  It may be unusual, but I don't think there is anything in the standard
>> that makes this a non-conforming extension.
>>
>
> C++ 7.1.3 Reserved identifiers
> "If the program declares or defines an identifier in a context in which it
> is reserved or defines a reserved identifier as a macro name, the behavior
> is undefined."


Yes. Which makes this a conforming extension. An implementation is free to
define this behavior however it likes, and Clang+LSan define it in a
useful, documented way to disable checking leaks in the program.


>
>
>
>>
>>     The checker I have is a strict warning flag I've been trying to
>>     encode the rules from the standard and in this case the diagnostic
>>     looks legitimate.
>>
>>
>> Ok, well, we disagree. ;]
>>
>> If the concern is that this is an ugly interface, I'm somewhat
>> sympathetic and we could explore better ways to expose this interface to
>> users, but I don't think this is incorrect as is, it just looks slightly
>> sub-optimal. And I don't think there is any standards problem here, as this
>> is and intentional extension provided by Clang+LSan.
>>
>> Now, one thing that I meant to ask for and didn't is to hid all of this
>> behind some macro which is used to enable LSan. If we're not compiling with
>> a compiler that supports this extension, it seems reasonable (if not likely
>> of practical importance) to avoid touching the reserved name.
>>
>
>
> Kostya's commit is the only instance I can find of this violation in
> dozens of large software projects. It's a first grader mistake (in the
> interface, not necessarily the commit), not something that should exist in
> a compiler toolchain codebase, thus my concern :-)
>

I don't understand. The sanitizers define a large number of interfaces in
the implementation's reserved names. Look at the headers installed in
lib/clang/3.5/include/sanitizer for examples. The only thing unusual here
is that rather than the user code using a reserved name which is defined by
the sanitizer runtimes, and here the sanitizer runtimes are using a
reserved name which is defined by the user code. However, that seems to be
the correct direction given the desired functionality. If you would like to
propose a way to have the user program call some function provided by the
sanitizer headers to accomplish this goal, by all means.


>
> If you really want to keep it however, let's compromise and you can put it
> behind a macro. I do however want to give a strong nudge to get this
> interface fixed. A single-underscore or no-underscore variant would do --
> nothing fancy needed.


This isn't something I "want" and we can't use those variants. This name
*needs* to be reserved so that it cannot conflict with user code. This name
is part of a compiler provided extension and we specifically want to
isolate it's name from user code.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140109/1db07353/attachment.html>