python3kgae added inline comments.

================
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:20
 #include "clang/Sema/Sema.h"
+#include "llvm/Frontend/HLSL/HLSLResource.h"
 
----------------
beanz wrote:
> python3kgae wrote:
> > python3kgae wrote:
> > > beanz wrote:
> > > > You need to add FrontendHLSL to the Sema/CMakeLists.txt file too.
> > > Good catch.
> > > Not sure why both local build and the pre-commit check cannot hit it. :(
> > I think the reason it works is that it only used the enum decl in the 
> > header, not anything which needs to link.
> > Do we need to add FrontendHLSL to CMakeLists in this case?
> The first header include added to a component should add the linkage 
> dependency (even if it isn't strictly needed).
> 
> It is too easy to add a linkage dependency later without realizing it isn't 
> already specified. Because of how static archive linking works you don't 
> necessarily notice the missing dependency because many of our tools link both 
> Sema and CodeGen where there is already a dependency.
Got it.
Updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136134/new/

https://reviews.llvm.org/D136134

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to