Bug 200188 – [WHLSL] Remove UnnamedType copy/move constructors and mark classes as final

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Saam Barati]

They don't make any sense anymore since UnnamedType is now ref counted. You should just take a ref to the thing instead.

[Saam Barati]

Created attachment 375009 [details]
patch

[EWS Watchlist]

Attachment 375009 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPointerType.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayType.h:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLTypeReference.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/247893/webkit

[Radar WebKit Bug Importer]

<rdar://problem/53628171>

[Darin Adler]

Comment on attachment 375009 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375009&action=review

> Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h:47
> +    WTF_MAKE_NONCOPYABLE(ArrayReferenceType);
> +    WTF_MAKE_NONMOVABLE(ArrayReferenceType);

Doesn’t make sense to me that you would ever need both of these. I don’t think that’s how C++ works. If you delete the copy constructor I don’t think you normally have to explicitly delete the move constructor too, for example.

[Saam Barati]

(In reply to Darin Adler from comment #5)
> Comment on attachment 375009 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=375009&action=review
> 
> > Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h:47
> > +    WTF_MAKE_NONCOPYABLE(ArrayReferenceType);
> > +    WTF_MAKE_NONMOVABLE(ArrayReferenceType);
> 
> Doesn’t make sense to me that you would ever need both of these. I don’t
> think that’s how C++ works. If you delete the copy constructor I don’t think
> you normally have to explicitly delete the move constructor too, for example.

Yeah good point. I was employing full paranoia without remembering that explicitly defining (a deleted or not) copy constructor removes the move constructor.

[Saam Barati]

I can prep a patch in a few minutes

[Saam Barati]

Created attachment 375258 [details]
followup patch

[Saam Barati]

Created attachment 375259 [details]
followup patch

[Saam Barati]

landed followup in
https://trac.webkit.org/changeset/248083/webkit