Introduce a WinRT utils library and "checked resources"#3350
Introduce a WinRT utils library and "checked resources"#3350DHowett-MSFT merged 4 commits intomasterfrom
Conversation
|
Also (!!!) |
9ee9298 to
81d3751
Compare
zadjii-msft
left a comment
There was a problem hiding this comment.
Two questions:
A: Is adding another project going to just absolutely kill our builds again?
B: Hypothetical scenario: we add another lib helper to the AppLib project, and that new lib also needs to do resource loading. Now our dependencies look like this:
- TerminalApp (dll)
- TerminalAppLib (lib)
- WinRTUtils (lib)
- SomeNewHelper (lib)
- WinRTUtils (lib)
- TerminalAppLib (lib)
Does this still work in that scenario?
I don't have any blocking issues (other than Michael's sln thing), but I'd love to hear answers before I signoff.
DHowett-MSFT
left a comment
There was a problem hiding this comment.
A single library with multiple statically linked libraries containing their own resources should be considered ill-formed, and this will be detected at link time (when there's duplicate definitions of all the functions in LibraryResources). If it's not detected at link time, it'll be detected at runtime when all of a single library's resources failfast on launch because one of the scopes won out.
|
I think our builds will be fine :) |
81d3751 to
dca08d8
Compare
This commit introduces a C++/WinRT utility library and moves ScopedResourceLoader into it. I decided to get uppity and introduce something I like to call "checked resources." The idea is that every resource reference from a library is knowable at compile time, and we should be able to statically ensure that all resources exist. This is a system that lets us immediately failfast (on launch) when a library makes a static reference to a resource that doesn't exist at runtime. It exposes two new (preprocessor) APIs: * RS_(wchar_t): loads a localizable string resource by name. * USES_RESOURCE(wchar_t): marks a resource key as used, but is intended for loading images or passing static resource keys as parameters to functions that will look them up later. Resource checking relies on diligent use of USES_RESOURCE() and RS_() (which uses USES_RESOURCE), but can make sure we don't ship something that'll blow up at runtime. It works like this: ** IN DEBUG MODE ** - All resource names referenced through USES_RESOURCE() are emitted alongside their referencing filenames and line numbers into a static section of the binary. That section is named .util$res$m. - We emit two sentinel values into two different sections, .util$res$a and .util$res$z. - The linker sorts all sections alphabetically before crushing them together into the final binary. - When we first construct a library's scoped resource loader, we iterate over every resource reference between $a and $z and check residency. ** IN RELEASE MODE ** - All checked resource code is compiled out. Fixes #2146.
dca08d8 to
7d06fb0
Compare
|
If you attempt to use a dynamic value in I think it's the best we can do for this diagnostic. |
|
The CI is failing because every project referenced by UnitTests_TerminalApp is being built twice. |
d39f1eb to
1eb8912
Compare
lol |
|
rofl. i know, right? we have a fix that i'm pulling into a separate PR. |
| <ClCompile> | ||
| <PrecompiledHeaderFile>pch.h</PrecompiledHeaderFile> | ||
| <AdditionalIncludeDirectories>..;$(OpenConsoleDir)\dep\jsoncpp\json;%(AdditionalIncludeDirectories);</AdditionalIncludeDirectories> | ||
| <AdditionalIncludeDirectories>..;$(OpenConsoleDir)\src\cascadia\WinRTUtils\inc;$(OpenConsoleDir)\dep\jsoncpp\json;%(AdditionalIncludeDirectories);</AdditionalIncludeDirectories> |
There was a problem hiding this comment.
Should this particular include project be a part of cppwinrt.build.pre.props common?
There was a problem hiding this comment.
I could go either way on this. I like the idea of having all C++/winrt projects able to access this ;P
There was a problem hiding this comment.
Decided to put it into cppwinrt pre props (@zadjii-msft may have a feeling about this)
There was a problem hiding this comment.
I have a feeling, the feeling is 😀
DHowett-MSFT
left a comment
There was a problem hiding this comment.
(i think i wrote comments but i have to write another comment to comment on these comments?)
1a9dfee to
681f334
Compare
|
Apart from the build working, what does this still need to land? Do we have consensus that |
|
fwiw it stands for "resource string" :P |
This commit introduces a C++/WinRT utility library and moves
ScopedResourceLoader into it. I decided to get uppity and introduce
something I like to call "checked resources." The idea is that every
resource reference from a library is knowable at compile time, and we
should be able to statically ensure that all resources exist.
PR Checklist
Details
This is a system that lets us immediately failfast (on launch) when a
library makes a static reference to a resource that doesn't exist at
runtime.
It exposes two new (preprocessor) APIs:
RS_(wchar_t): loads a localizable string resource by name.USES_RESOURCE(wchar_t): marks a resource key as used, but is intendedfor loading images or passing static resource keys as parameters to
functions that will look them up later.
Resource checking relies on diligent use of
USES_RESOURCE()andRS_()(which uses
USES_RESOURCE), but can make sure we don't ship somethingthat'll blow up at runtime.
It works like this:
IN DEBUG MODE
All resource names referenced through
USES_RESOURCE()are emittedalongside their referencing filenames and line numbers into a static
section of the binary.
That section is named
.util$res$m.We emit two sentinel values into two different sections,
.util$res$aand
.util$res$z.The linker sorts all sections alphabetically before crushing them
together into the final binary.
When we first construct a library's scoped resource loader, we
iterate over every resource reference between
$aand$zand checkresidency.
IN RELEASE MODE
Fixes #2146.
Macros are the only way to do something this cool, incidentally.
Validation Steps Performed
Made references to a bunch of bad resources, tried to break it a lot.
It looks like this when it fails:
App.cpp