[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