On 09/01/2014 11:36, Chandler Carruth wrote:
On Thu, Jan 9, 2014 at 3:15 AM, Alp Toker <[email protected]
<mailto:[email protected]>> wrote:
__builtin functions are introduced by the implementation (either
the compiler or system header declarations) so there's no problem.
Sanitizer is a system library so it's free to use the identifiers
it wants.
The sanitizers aren't system libraries as I define them (things in
/usr/include or /usr/lib). They're compiler runtime libraries. They
get installed in lib/clang/... along with other compiler resources.
They are tightly coupled and version locked with the compiler itself
as they interface with compiler generated instrumentation in most
cases. The leak sanitizer is unusual in that it *could* be separated
into something other than the compiler runtime libraries, but we
explicitly chose not to as a design goal: we want to be able to offer
it coupled with address sanitizer.
Also, system libraries aren't free to use reserved names... so maybe
you meant something different from what is commonly called a system
library on linux/mac?
s/libraries/headers/ clearly.
TableGen, on the other hand, is not a part of the implementation.
No one is claiming that it is.
Can you clarify exactly what the error is and why? It's
possible we could provide a macro of some kind in the
sanitizer headers to bundle this functionality up if we need
some special marking of this for checkers, but without more
information its hard to hell what the actual problem is.
These two commits _introduced_ names in the reserved namespace.
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.
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."
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 :-)
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.
Alp.
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
