[PATCH] D112663: [clang-repl] Allow Interpreter::getSymbolAddress to take a mangled name.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/Interpreter/Interpreter.h:70
   llvm::Expected<llvm::JITTargetAddress>
-  getSymbolAddress(llvm::StringRef UnmangledName) const;
+  getSymbolAddress(llvm::StringRef Name, bool IsMangled = false) const;
 };
----------------
I find this flag name to be very confusing, given what "mangling" usually means within Clang. I think `IsMangled = false` means "this is a symbol name of the same kind that Clang would emit in IR", and `IsMangled = true` means "this is a symbol name of the same kind that would appear in an object file's symbol table" -- and the former is a mangled name in the sense in which the term is used in Clang.

Perhaps `enum { IRName, LinkerName }` instead of a bool flag would be clearer? But I'm not sure whether this flag is motivated given that your test case below wants / is testing the `IsMangled = false` behavior.

I wonder if a better interface here generally would be:

```
llvm::Expected<llvm::JITTargetAddress> getSymbolAddress(GlobalDecl GD) const;
```

... which could internally form the proper symbol name for the given `GlobalDecl` (or perhaps reuse `CodeGen`'s decl -> mangling map rather than redoing the name mangling step).


================
Comment at: clang/unittests/Interpreter/InterpreterTest.cpp:131-138
+static std::string MangleName(NamedDecl *ND) {
+  ASTContext &C = ND->getASTContext();
+  std::unique_ptr<MangleContext> MangleC(C.createMangleContext());
+  std::string mangledName;
+  llvm::raw_string_ostream RawStr(mangledName);
+  MangleC->mangleName(ND, RawStr);
+  return RawStr.str();
----------------
This function produces names that are not mangled in the sense that `getSymbolAddress`'s `IsMangled = true` means. I would expect this test to fail on MacOS for that reason.


Repository:
  rC Clang

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

https://reviews.llvm.org/D112663