Bug 200870 - [WHLSL] Generated MSL code should be indented properly to ease reading while debugging
Summary: [WHLSL] Generated MSL code should be indented properly to ease reading while ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-18 14:19 PDT by Sam Weinig
Modified: 2019-08-19 22:48 PDT (History)
10 users (show)

See Also:


Attachments
Patch (97.40 KB, patch)
2019-08-18 14:30 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
before.txt (30.95 KB, text/plain)
2019-08-18 14:30 PDT, Sam Weinig
no flags Details
after.txt (44.58 KB, text/plain)
2019-08-18 14:31 PDT, Sam Weinig
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2019-08-18 14:19:14 PDT
[WHLSL] Make generated Metal code should be indented properly to ease reading while debugging
Comment 1 Sam Weinig 2019-08-18 14:30:13 PDT
Created attachment 376649 [details]
Patch
Comment 2 Sam Weinig 2019-08-18 14:30:44 PDT
Created attachment 376650 [details]
before.txt
Comment 3 Sam Weinig 2019-08-18 14:31:41 PDT
Created attachment 376651 [details]
after.txt
Comment 4 Saam Barati 2019-08-19 18:14:48 PDT
Comment on attachment 376649 [details]
Patch

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

r=me

> Source/WTF/wtf/text/StringConcatenate.h:262
> +template<unsigned N>
> +struct Indentation {
> +    unsigned operator++() { return ++value; }
> +    unsigned operator++(int) { return value++; }
> +    unsigned operator--() { return --value; }
> +    unsigned operator--(int) { return value--; }
> +
> +    unsigned value { 0 };
> +};
> +
> +
> +template<unsigned N>
> +struct IndentationScope {
> +    IndentationScope(Indentation<N>& indentation)
> +        : m_indentation(indentation)
> +    {
> +        ++m_indentation;
> +    }
> +    ~IndentationScope()
> +    {
> +        --m_indentation;
> +    }
> +
> +    Indentation<N>& m_indentation;
> +};

Sort of a nit, so maybe it doesn't matter, but I kind of think it's nicer to localize ++/-- to increment and decrement by N, instead of making users of Indentation have to consider what N is.

For example, below, the StringTypeAdaptor for Indentation has to know what N is (in length and writeTo). If we had other users in the future, they'd also have to concern themselves with what N is and what the "value" field is, instead of just being concerned with what "value" is.
Comment 5 WebKit Commit Bot 2019-08-19 22:47:44 PDT
Comment on attachment 376649 [details]
Patch

Clearing flags on attachment: 376649

Committed r248892: <https://trac.webkit.org/changeset/248892>
Comment 6 WebKit Commit Bot 2019-08-19 22:47:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-08-19 22:48:19 PDT
<rdar://problem/54499451>