SouraVX marked 2 inline comments as done. SouraVX added inline comments.
================ Comment at: include/clang/AST/DeclCXX.h:2931 + lang_cxx_11 = /* DW_LANG_C_plus_plus_11 */ 0x001a, + lang_cxx_14 = /* DW_LANG_C_plus_plus_14 */ 0x0021 }; ---------------- aprantl wrote: > SouraVX wrote: > > aprantl wrote: > > > SouraVX wrote: > > > > aprantl wrote: > > > > > I understand that DWARF does not define a C++17 language constant, > > > > > but in the AST I fell like we should represent it, even if it is > > > > > being lowered into C++14 for the debug info. > > > > It's represented in AST as enum in LangStandard.h file. We didn't add > > > > it here, as DWARF5 has no opcode for C++17. If we add it here some > > > > opcode, this might cause conflict with future upcoming DWARF standard. > > > Since these are the DWARF DW_lang constants, we shouldn't repeat their > > > numeric values here. Can we write this as > > > `lang_c = llvm::dwarf::DW_LANG_C`, etc? > > We can do this, but not sure whether, we should do this in AST ? > > Since it's a language specific part, if we integrate this from DWARF > > opcodes it will unnecessary pollute AST ? Other language opcodes will also > > be visible here. > I'm not sure I understand the concern here. Are you worried about #Including > "llvm/BinaryFormat/DWARF.h" and making these definitions visible to users of > DeclCXX.h? All symbols there are in a separate namespace. Thanks @aprantl for your review. I was sceptical about, introducing DWARF related changes to AST, however you're correct, those are just language enums. This will be heplfull in future, while adding language tag. If more C/C++ versions become part of DWARF standard. Will address this change in next revision. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67613/new/ https://reviews.llvm.org/D67613 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits