rnk added inline comments.
================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 + llvm::APSInt Value = Enum->getInitVal(); + Value.setIsSigned(IsSigned); + Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); ---------------- dblaikie wrote: > rnk wrote: > > dblaikie wrote: > > > Is the value already signed appropriately? > > > > > > Removing this line of code (and the `bool IsSigned` variable, so it > > > doesn't break `-Werror=unused-variable`) doesn't cause any tests to fail, > > > that I can see. > > I'm afraid it might not be NFC. I took the cautious approach of trying to > > leave things exactly as they were. Enums in C can have surprisingly > > different behavior, IIRC. > I'd rather not leave in haunted-graveyard-y code like that. > > Could we assert that Value.getIsSigned == IsSigned? Then if there's a case > where it isn't we'll have a test case/justification for the code? I convinced myself that the signed-ness of the APSInt in the EnumConstantDecl always matches by looking at this code here: https://github.com/llvm/llvm-project/blob/aee8457b8d4123d087c45aef95d14f24934fed53/clang/lib/Sema/SemaDecl.cpp#L18379 So far as I can tell, we always run that code, no matter whether LangOpts.CPlusPlus is set or not. That's enough for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106585/new/ https://reviews.llvm.org/D106585 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
