[PATCH] D82160: [lldb][PDB] Constexpr static member values as AST literals

Sat Jun 20 23:57:57 PDT 2020

jackoalan marked 4 inline comments as done.
jackoalan added a comment.

Thank you @teemperor, your comments and changes in D81471 <https://reviews.llvm.org/D81471> are very informative. I didn't realise DWARF had similar issues with static const members.

Curiously, I am able to resolve direct variable expressions without your `ASTResultSynthesizer` changes. I'm guessing this is due to differences in the MS linking convention for the IR module.

If you're interested, here are lldb.expr logs comparing member evaluation with and without the synthesizer change:
without: https://gist.github.com/jackoalan/f70f223c5e9ec25d6c8e3a5e09940a71
with: https://gist.github.com/jackoalan/e33c878c545c5b270e6f3da44ef57320

Your changes are definitely preferred. The IR execution takes a much more roundabout approach originally, and the change noticeably speeds up evaluation.

================
Comment at: lldb/test/Shell/SymbolFile/PDB/Inputs/AstRestoreTest.cpp:27
+  static constexpr float ClassStaticConstexprFloat = 9.f;
+  static constexpr double ClassStaticConstexprDouble = 10.0;

----------------
teemperor wrote:
> You might want to also add some static const (not `constexpr`) integer and enum members to this test.
> 
> Also I'm curious what happens if you put a `long double` as a member? From what I can see the expected behavior is that PDB will not emit that as a constant (as the current code asserts if it encounters `long double` anywhere).
`long double` is aliased to `double` according to MS docs. The constant can be extracted in the same manner.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82160