Bug 192826 - [WHLSL] Add a Visitor class
Summary: [WHLSL] Add a Visitor class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on: 192991
Blocks: 193007 193360
  Show dependency tree / graph
 
Reported: 2018-12-18 13:20 PST by Myles C. Maxfield
Modified: 2019-01-11 18:47 PST (History)
13 users (show)

See Also:

Attachments
Patch (79.52 KB, patch)
2018-12-18 13:21 PST, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (79.43 KB, patch)
2018-12-20 23:13 PST, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (92.13 KB, patch)
2018-12-21 18:56 PST, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (48.06 KB, patch)
2019-01-11 00:12 PST, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (46.94 KB, patch)
2019-01-11 10:50 PST, Myles C. Maxfield 		dino: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2018-12-18 13:20:03 PST 
Add a Visitor class
Comment 1 Myles C. Maxfield 2018-12-18 13:21:46 PST 
Created attachment 357603 [details]
Patch
Comment 2 Myles C. Maxfield 2018-12-20 23:13:16 PST 
Created attachment 357930 [details]
Patch
Comment 3 Myles C. Maxfield 2018-12-21 18:56:41 PST 
Created attachment 358011 [details]
Patch
Comment 4 Myles C. Maxfield 2019-01-11 00:12:00 PST 
Created attachment 358882 [details]
Patch
Comment 5 Myles C. Maxfield 2019-01-11 10:50:20 PST 
Created attachment 358911 [details]
Patch
Comment 6 Robin Morisset 2019-01-11 15:16:10 PST 
Comment on attachment 358911 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:621
> +void Visitor::checkErrorAndVisit(Program& program)

These look very repetitive. Would the following fail for some reason?
```
template<typename T> void Visitor::checkErrorAndVisist(T& x)
{
  if (!error)
    visit(x);
}
```
Comment 7 Dean Jackson 2019-01-11 16:34:36 PST 
Comment on attachment 358911 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

2019

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:105
> +    // These visiting functions might add new global statements, so don't use foreach syntax.

:(

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:357
> +    WTF::visit(WTF::makeVisitor([&](AST::NumThreadsFunctionAttribute& numThreadsFunctionAttribute) {
> +        checkErrorAndVisit(numThreadsFunctionAttribute);
> +    }), functionAttribute);

Why not call it directly since there is only one object to visit?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.h:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

2019

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.h:179
> +    void checkErrorAndVisit(Program&);

Could these be private/protected?
Comment 8 Myles C. Maxfield 2019-01-11 17:35:21 PST 
Comment on attachment 358911 [details]
Patch

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

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:621
>> +void Visitor::checkErrorAndVisit(Program& program)
> 
> These look very repetitive. Would the following fail for some reason?
> ```
> template<typename T> void Visitor::checkErrorAndVisist(T& x)
> {
>   if (!error)
>     visit(x);
> }
> ```

Yes! Good idea.
Comment 9 Myles C. Maxfield 2019-01-11 18:46:23 PST 
Committed r239893: <https://trac.webkit.org/changeset/239893>
Comment 10 Radar WebKit Bug Importer 2019-01-11 18:47:38 PST 
<rdar://problem/47227226>