akhuang marked an inline comment as done.
akhuang added inline comments.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2263
if (DebugKind == codegenoptions::DebugInfoConstructor &&
- !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) {
- for (const auto *Ctor : CXXDecl->ctors()) {
+ !CXXDecl->isLambda() && !CXXDecl->isLiteral() &&
+ !isClassOrMethodDLLImport(CXXDecl))
----------------
dblaikie wrote:
> rnk wrote:
> > I'm not sure `isLiteral` is quite the right check. I think there are some
> > ways to:
> > - have a type that is not literal
> > - construct it at compile time with the constexpr constructor
> > - skip object destruction
> >
> > I came up with this example:
> > ```
> > struct ConstexprCtor {
> > constexpr ConstexprCtor(int a, int b) : a(a + 1), b(b + 1) {}
> > ~ConstexprCtor();
> > int a, b;
> > };
> >
> > [[clang::no_destroy]] ConstexprCtor static_gv{1, 2};
> > ```
> >
> > I tried to find other ways to construct a class in a constexpr context
> > without running the destructor, but wasn't able to.
> >
> > It would also be interesting to know if StringRef and ArrayRef are literal
> > or not. Those seem like types that we would want to apply this optimization
> > to, and they have constexpr constructors. Maybe we can find a way to apply
> > the optimization by figuring out if a constexpr constructor has been
> > evaluated. You could look at who calls `Sema::MarkFunctionReferenced` and
> > see if there are any callbacks into the ASTConsumer around there.
> Was (well did, and then realized you'd covered it) going to say the same
> thing about "is literal type" probably being too narrow here. Testing for the
> presence of a non-copy/move ctor constexpr ctor I think would be the right
> test.
>
> I'd say it'd be best to fix this broadly first, then in a separate patch/work
> try to find a way to get some types with constexpr ctors back in - as I think
> that's going to be extra tricky with optimizations, linkages, etc. Maybe not
> - but I think there's enough to discuss there that we shouldn't tie that
> discussion together with this correctness issue.
Sounds good! And yeah, I sort of started looking into how to not ignore all
constexpr ctor types, makes sense to put it in a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77432/new/
https://reviews.llvm.org/D77432
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
