Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WASDK 1.4.231008000 not including <Unknwn.h> before "winrt/base.h" #9014

Closed
StevoSM opened this issue Oct 25, 2023 · 11 comments
Closed

WASDK 1.4.231008000 not including <Unknwn.h> before "winrt/base.h" #9014

StevoSM opened this issue Oct 25, 2023 · 11 comments
Labels
area-XamlCompiler bug Something isn't working closed-Fixed Described behavior has been fixed. needs-triage Issue needs to be triaged by the area owners team-Markup Issue for the Markup team
Milestone

Comments

@StevoSM
Copy link

StevoSM commented Oct 25, 2023

I'm just getting started with the WASDK and WinUI 3 so I'm not sure how things should be configured. We have been doing WinUI application development for a couple years, starting with WinUI 2 and most recently transitioning to WinUI 3 1.3.x.

Our project does not use precompiled headers. We have had no problem with building the Project.

However, in updating to 1.4.x, we are getting build errors because "winrt/base.h" is being included before <Unknwn.h>.

We are able to get things to compile by manually adding it in the generated app namespace header. However, since this file is re-generated periodically, it becomes very annoying to build, only to have to update and build again.

#include <Unknwn.h>
#include "winrt/base.h"

We would appreciate direction on how to properly configure our Project so we don't need to do this manually. For now, we have reverted to 1.3.x.

@DarranRowe
Copy link

DarranRowe commented Oct 25, 2023

You shouldn't have to think of this that strictly. What this really means is that you need to have the classic COM IUnknown defined before including the first of any of the C++/WinRT headers.
One of my test projects also doesn't use precompiled headers, however, I set it up to have a single header including all of the common headers that I am likely to use in this project.

//commoninclude.h
#pragma once

#include <sdkddkver.h>

#include <Windows.h>
#include <windowsx.h>
#include <appmodel.h>
#include <ShlObj.h>
#include <dxgi1_6.h>
#include <d3d11_4.h>
#include <d2d1_3.h>
#include <dwrite_3.h>

#include <propkey.h>
#include <propsys.h>
#include <string>
#include <source_location>

#include <cinttypes>
#include <cstdint>

#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>
#include <winrt/Windows.UI.Composition.h>
#include <winrt/Windows.UI.Composition.Desktop.h>
#include <winrt/Windows.UI.ViewManagement.h>

#include <winrt/Microsoft.UI.Windowing.h>
#include <winrt/Microsoft.Windows.AppNotifications.h>
#include <winrt/Microsoft.Windows.AppNotifications.Builder.h>

#include <windows.foundation.h>
#include <windows.foundation.collections.h>
#include <windows.applicationmodel.h>
#include <windows.management.deployment.h>

#include <wil/result_originate.h>
#include <wil/cppwinrt.h>
#include <wil/cppwinrt_helpers.h>
#include <wil/resource.h>
#include <wil/wrl.h>
#include <wrl.h>

With this include order, winrt/base.h sees IUnknown as defined. This happens because Windows.h includes ole2.h by default, and this in turn includes unknwnbase.h. In other words, just including Windows.h before the first C++/WinRT header will normally satisfy this. If you are using #define WIN32_LEAN_AND_MEAN, then you must include any of the basic COM headers after Windows.h but before the first C++/WinRT header.

@evelynwu-msft
Copy link
Contributor

@StevoSM Did the version of C++/WinRT that your project is using also get downgraded when you rolled back to WinAppSDK 1.3?

@evelynwu-msft
Copy link
Contributor

Additionally, can you share a minimal repro project? I'm confused as to why downgrading the version of WinAppSDK being used would affect whether or not your project successfully compiles since to the best of my knowledge this area of the generated code has not been touched in a very long time.

@StevoSM
Copy link
Author

StevoSM commented Oct 26, 2023

To provide more context, here are some more details:

The issue arises when compiling the generated XamlTypeInfo.g.cpp.

Using WASDK 1.4, XamlTypeInfo.g.cpp starts with:

#include <memory>

#if __has_include(<winrt/AppNamespace.h>)
#include <winrt/AppNamespace.h>
#endif
#if __has_include(<winrt/OtherNamespace.h>)
#include <winrt/OtherNamespace.h>
#endif
#if __has_include(<winrt/Microsoft.UI.Xaml.Controls.h>)
#include <winrt/Microsoft.UI.Xaml.Controls.h>
#endif
#if __has_include(<winrt/Windows.Foundation.Collections.h>)
#include <winrt/Windows.Foundation.Collections.h>
#endif

#include "XamlTypeInfo.xaml.g.h"
...

winrt/AppNamespace.h starts with:

#include "winrt/base.h"
...

XamlTypeInfo.xaml.g.h starts with:

#pragma once

#include <functional>
#include <map>
#include <vector>
#include <unknwn.h>
...

However, using WASDK 1.3, XamlTypeInfo.g.cpp starts with:

#include <memory>

#include "XamlTypeInfo.xaml.g.h"
...

We can see from the above that 1.4 is inserting additional includes between <memory> and XamlTypeInfo.xaml.g.h which end up including winrt headers before "unknwn.h" is set to be included.

@DarranRowe I'm very glad to update my app's includes as needed. Do you know how to make the needed changes that would affect the generated files?

@evelynwu-msft All of this is with the latest C++/WinRT - 2.0.230706.1. There's quite a lot in the current Project - I'm not sure how much effort it would take to create a minimal repo project and I expect that may take considerable time.

@DarranRowe
Copy link

DarranRowe commented Oct 26, 2023

This is a Xaml compiler issue, since that is what generates XamlTypeInfo.g.cpp. This means that the minimal reproducible example is the basic C++ WinUI 3 project template followed by disabling the precompiled header in the project properties.
Honestly, given the situation, your best options are to either enable precompiled headers for now, or use the force include option.

@evelynwu-msft
Since the Xaml compiler source was made available, this problem is in CppWinRT_TypeInfoPass2.cs. Specifically the TransformText method of CppWinRT_TypeInfoPass2. It first generates the inclusion of the precompiled header followed by the inclusion of <memory>, it then includes C++/WinRT projection headers, next is the include guard and inclusion for the incremental type info option, finally XamlTypeInfo.xaml.g.h is included. This order is the problem since without precompiled headers enabled, this guarantees that unkwn.h is included after the C++/WinRT headers.

@bpulliam bpulliam added the team-Markup Issue for the Markup team label Oct 26, 2023
@StevoSM
Copy link
Author

StevoSM commented Oct 27, 2023

@DarranRowe Thanks for sleuthing that out. You mentioned you are not using precompiled headers, but are not seeing this same issue. Are you not using XAML in your project, or how is it working for you?

I actually turned off precompiled headers as Visual Studio was flagging that it was not included in most of the source files, which is intentional as most of the source files are cross-platform C++ files and precompiled headers are not used on the other platforms.

To move this forward for us, we went with the force include, which also required an additional #define in the Project Settings to turn off the min/max macros.

@evelynwu-msft evelynwu-msft added bug Something isn't working area-XamlCompiler labels Oct 27, 2023
@evelynwu-msft
Copy link
Contributor

@DarranRowe That's awesome that you were able to identify the root cause in the source code!

@StevoSM Thanks for bringing this to our attention. We'll get this regression addressed as soon as we can.

@StevoSM
Copy link
Author

StevoSM commented Nov 1, 2023

@evelynwu-msft Sounds great! Thanks so much!

@llongley llongley closed this as completed Feb 9, 2024
@llongley llongley added the closed-Fixed Described behavior has been fixed. label Feb 9, 2024
@StevoSM
Copy link
Author

StevoSM commented Feb 12, 2024

@llongley Can you speak to what release contains the fix?

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Feb 12, 2024
@evelynwu-msft
Copy link
Contributor

evelynwu-msft commented Feb 13, 2024

@StevoSM The fix will be included in the WinAppSDK 1.5 release.

@StevoSM
Copy link
Author

StevoSM commented Feb 22, 2024

@evelynwu-msft Great! Thanks!

@bpulliam bpulliam added this to the WinAppSDK 1.5 milestone Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-XamlCompiler bug Something isn't working closed-Fixed Described behavior has been fixed. needs-triage Issue needs to be triaged by the area owners team-Markup Issue for the Markup team
Projects
None yet
Development

No branches or pull requests

5 participants