martong marked 3 inline comments as done.
martong added a comment.
In D122805#3419129 <https://reviews.llvm.org/D122805#3419129>, @steakhal wrote:
> Please add a test case where the class is trivial but the global variable of
> such is non-const, thus the initialized expression is not imported.
I've added a new case, but that is not very meaningful, because
`clang-extdef-mapping` is not capable of emitting the USR for the non-const
variable.
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:112
-// Returns true if the variable or any field of a record variable is const.
-bool containsConst(const VarDecl *VD, const ASTContext &ACtx);
+/// Returns true if it makes sense to import a foreign variable declaration.
+/// For instance, we don't want to import variables that have non-trivial types
----------------
steakhal wrote:
> definition
thx, fixed.
================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:195
+bool shouldImport(const VarDecl *VD, const ASTContext &ACtx) {
+
CanQualType CT = ACtx.getCanonicalType(VD->getType());
----------------
steakhal wrote:
> extra blank line
Thanks!
================
Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:128
SCNest extSCN = {.scn = {.a = 9}};
-SCNest::SCN extSubSCN = {.a = 1};
+extern SCNest::SCN const extSubSCN = {.a = 1};
struct SCC {
----------------
steakhal wrote:
> AFAIK you don't need the `extern` here, since **this is** the definition of
> that global, not only a declaration.
> Similarly applies to the rest of the cases.
Actually, linkage of `const` variable definitions are internal in C++, unless
the `extern` modifier is provided. So, we need it.
If it was a non-const then the definition's linkage would be external.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122805/new/
https://reviews.llvm.org/D122805
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
