[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX marked 7 inline comments as done.
SouraVX added inline comments.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1610
+  if (const auto *CXXC = dyn_cast<CXXConstructorDecl>(Method)) {
+    if (CXXC->getCanonicalDecl()->isDeleted())
+      SPFlags |= llvm::DISubprogram::SPFlagDeleted;
----------------
aprantl wrote:
> can you factor this block out into a static function or lambda, since it is repeated three times?
Refactored this into lambda, with additional return statement after every check, since every attribute is mutually exclusive. No need to check others once an attribute is set.


================
Comment at: clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp:6
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone -dwarf-version=5 \
+// RUN: | FileCheck %s -check-prefix=ATTR
----------------
aprantl wrote:
> the -dwarf-version flag should be unnecessary now, right?
Thanks for correcting again! Removed unnecessary flags from test cases.


================
Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)
----------------
aprantl wrote:
> Since these are all mutually exclusive, it might make sense to do the same thing as we did for virtuality above and thus save a bit or two.
Had this in mind when I added 4 flags but couldn't able to solve this back then so I put this. 

I tried refactoring this couple times based on your comments with results in failure. Virtuality and Defaulted member have same encodings "00-01-02". 
My approach was conflicting with Virtuality attributes, Since Virtuality here is already using 2 bits of LSB. 
Could you please suggest or hints to tackle this?


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

https://reviews.llvm.org/D68117