On Thu, Jan 9, 2014 at 6:57 PM, Alp Toker <[email protected]> wrote: > > On 09/01/2014 12:13, Chandler Carruth wrote: > > > On Thu, Jan 9, 2014 at 4:07 AM, Alp Toker <[email protected]> wrote: > >> I understand that, but the question is -- do you think that usage >>> pattern will be prevalent? Is it worth putting an external definition in >>> the binary *always* just to catch the case where we do a link-time-only >>> flag? >>> >>> Put another way, is it really too burdensome to say that LSan does >>> require a compile time flag in order to support some usage patterns? >>> >> >> Who controls / defines the interface? >> > > The sanitizers. > > > Can you report a bug with them? >
That's us (no need to file a separate bug unless you want to) > They need to provide hooks that can be implemented cleanly. > > It's not reasonable to force something like this on the entire community > just because your internal schedule demands it. > > There are plenty of other checkers and analyzers that work fine without > forcing problematic code, and that can be disabled without impacting > standard builds, so it's not a big problem for the community if this takes > a few days to get resolved out of tree. > > To be clear, the commits not only introduce dead code, but code that > actively causes build issues in strict setups because it's not compliant > with the C++ specification and steps on the reserved namespace. > > Furthermore, the references I could find to this group of sanitizer > functions doesn't inspire confidence that they were thought through as > thoroughly as you suggest: > > kcc commented on this revision. > I am actually *not a big fan of this patch*. > We know *only one use case* for it, and I really hope we can find a > cleaner solution. > > I remember my concerns. Now we know several use cases and we did not find a cleaner solution yet. > > samsonov requested changes to this revision. > I agree with Kostya that *we should avoid adding this* interface function > if possible. > > > > >> >> This looks like a spectacular thinko rather than being something >> intentional -- is it possible to fix it to drop one or more underscores >> instead of devising workarounds? >> > > I have no idea what you mean. This was a well considered change, and > part of a design that has been developed over quite some time on the lists. > None of these are workarounds? > > >> >> There's never a valid reason to require user code to define reserved >> names (although it's sometimes OK for user code to _use_ reserved names). >> > > You keep making this assertion, but I still don't find any backing for > it in the standard. I'm happy to check with some of my fellow members of > the committee, but I would be surprised if they drew the distinction you > are drawing here. > > > I'm not making an assertion. The standard is very clear on this: > > > *17.6.4.3 Reserved names [reserved.names]* > > If a program declares or defines a name in a context where it is reserved, > other than as explicitly allowed by this Clause, its *behavior is > undefined*. > > *17.6.4.3.2 Global names [global.names]* > > Certain sets of names and function signatures are always reserved to the > implementation: > > > - *Each name that contains a double underscore __* or begins with an > underscore followed by an uppercase letter (2.12) *is reserved to the > implementation for any use*. > - Each name that begins with an underscore is reserved to the > implementation for use as a name in the global namespace. > > > > > >> >> I'm heading out for a couple of hours, please gate this behind a macro or >> revert until a resolution is found. > > > Alp, I really don't think this is reasonable. This change, and the lead up > to the change, were discussed on the list with code review. I understand > you have some high level design concerns, but there doesn't appear to be > broad agreement with them and I don't think it is reasonable to block > progress here while we sort them out. > > I'm happy to continue this discussion, but I do not think that this > needs to be reverted, and I do not think we need to block progress > > > I've just discussed with colleagues as well as a clang frontend developer > (all of whom are pretty keyed in on the matter) and there's broad agreement > that these changes are not wanted in their current form. > > Moreover, it's blocking work on a practical level by triggering legitimate > build issues in at least one configuration. Removing the function doesn't > break anything and resolves the issues. I don't see the problem here? > Will you get unblocked if we put this code under #ifdef LEAK_SANITIZER ? Alternatively you may probably build the code with -D __lsan_is_turned_off=something_else Do you have suggestions how to improve the interface w/o risking to clash with user-defined names? --kcc > > Anyway, I'm returning to the office to deal with this and I'm disappointed > you've tried to push it through unilaterally with disregard for anyone > outside of your own circle. > > > Alp. > > -- http://www.nuanti.com > the browser experts > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
