[PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 16 10:55:15 PDT 2016


On Wed, Mar 16, 2016 at 10:52 AM, don hinton <hintonda at gmail.com> wrote:

> -Werror clean is great.
>
> If you could add -Wdeprecated, then we wouldn't need to delete them -- the
> warning is only issued with -Wdeprecated is passed.
>

Right, that's what I'm saying - add a fixme so that once we turn on the
deprecated warning (which will be an error, because enough of us use
-Werror) we should remove these deleted ops & just rely on the
warning/error.


>
>
> On Wed, Mar 16, 2016 at 1:49 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Wed, Mar 16, 2016 at 10:19 AM, don hinton <hintonda at gmail.com> wrote:
>>
>>> The current behavior, albeit deprecated, is to implicitly define these.
>>> Therefore, it would be incorrect not to delete them if the implicit
>>> versions don't do the right thing.
>>>
>>> I'd be happy to add a FIXME, but I doubt they will ever be removed.  At
>>> best, they'd be #ifdef'd away for some future compiler that no longer
>>> implicitly defines them.
>>>
>>> Just not sure it's worth it.  Deleting them will be valid no matter what
>>> the future holds.
>>>
>>
>> Sure, but it's a lot of extra deletes (if you search the mailing list for
>> "author: dblaikie" and "deprecated" you'll find many other instances I
>> cleaned up, in which we'd have to explicitly delete these ops if that's
>> going to be our approach)
>>
>> I don't think we'd only ifdef them away. Keeping the clang build -Werror
>> clean is a pretty well established bar, so if anyone accidentally
>> introduced a call to any of these, we'd find/fix it pretty quickly as we do
>> other errors/mistakes, even when they're not hard language-required errors.
>>
>>
>>>
>>> On Wed, Mar 16, 2016 at 12:56 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Mar 16, 2016 at 7:54 AM, don hinton via cfe-commits <
>>>> cfe-commits at lists.llvm.org> wrote:
>>>>
>>>>> hintonda updated this revision to Diff 50823.
>>>>> hintonda added a comment.
>>>>>
>>>>> Address FIXME now that Sema::LookupInlineAsmField() has been fixed.
>>>>>
>>>>>
>>>>> http://reviews.llvm.org/D18123
>>>>>
>>>>> Files:
>>>>>   include/clang/AST/UnresolvedSet.h
>>>>>   include/clang/Sema/Lookup.h
>>>>>
>>>>> Index: include/clang/Sema/Lookup.h
>>>>> ===================================================================
>>>>> --- include/clang/Sema/Lookup.h
>>>>> +++ include/clang/Sema/Lookup.h
>>>>> @@ -185,6 +185,9 @@
>>>>>        Shadowed(false)
>>>>>    {}
>>>>>
>>>>> +  LookupResult(const LookupResult &) = delete;
>>>>> +  LookupResult & operator=(const LookupResult &) = delete;
>>>>>
>>>>
>>>> Not sure how much to bother explicitly deleting ops like this if
>>>> eventually the -Wdeprecated warning will just catch it that way. Maybe
>>>> leave a "FIXME: Remove these once the warning for deprecated copy ops is
>>>> enabled"?
>>>>
>>>>
>>>>> +
>>>>>    ~LookupResult() {
>>>>>      if (Diagnose) diagnose();
>>>>>      if (Paths) deletePaths(Paths);
>>>>> Index: include/clang/AST/UnresolvedSet.h
>>>>> ===================================================================
>>>>> --- include/clang/AST/UnresolvedSet.h
>>>>> +++ include/clang/AST/UnresolvedSet.h
>>>>> @@ -59,8 +59,11 @@
>>>>>    // UnresolvedSet.
>>>>>  private:
>>>>>    template <unsigned N> friend class UnresolvedSet;
>>>>> -  UnresolvedSetImpl() {}
>>>>> -  UnresolvedSetImpl(const UnresolvedSetImpl &) {}
>>>>> +  UnresolvedSetImpl() = default;
>>>>> +  UnresolvedSetImpl(const UnresolvedSetImpl &) = default;
>>>>> +  UnresolvedSetImpl(UnresolvedSetImpl &&) = default;
>>>>> +  UnresolvedSetImpl& operator=(const UnresolvedSetImpl &) = default;
>>>>> +  UnresolvedSetImpl& operator=(UnresolvedSetImpl &&) = default;
>>>>>
>>>>>  public:
>>>>>    // We don't currently support assignment through this iterator, so
>>>>> we might
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160316/7cd7c1b4/attachment-0001.html>


More information about the cfe-commits mailing list