[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added inline comments.


================
Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:21
+  StringRef InFile;
+  StringRef Format;
+  std::set<std::string> ParsedTemplates;
----------------
compnerd wrote:
> An enumeration may be nicer.
For now the format goes into the first line of the yaml (in both formats) and ends up as "--- ! <Format>\n".
I'd like to keep it this way for now. 


================
Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:39
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+    if (!(RDO & FromTU))
+      return true;
----------------
compnerd wrote:
> I tend to prefer `if (RDO & ~FromTU)`
0 != (RDO & 0x1) vs (RDO & 0xE)

I'm not sure if these are logically equivalent


================
Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:72
+             VD->getParentFunctionOrMethod() == nullptr))
+          return true;
+      if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
----------------
compnerd wrote:
> If it is a `VarDecl`, it cannot be a `FunctionDecl`.  I think that this can be simplified a bit as:
> 
> ```
> if (const auto *VD = dyn_cast<VarDecl>(VD))
>   return VD->getStorageClass() == StorageClass::SC_Extern ||
>       VD->getStorageClass() == StorageClass::SC_Static ||
>       VD->getParentFunctionOrMethod() == nullptr;
> ```
I don't understand. Here I remember wanting to bail if VD->getParentFunctionOrMethod() == nullptr and the vardecl was marked static. 


================
Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:77
+          return true;
+        if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
+          if (const auto *RC = dyn_cast<CXXRecordDecl>(MD->getParent())) {
----------------
compnerd wrote:
> I think that flipping this around would be nicer.
> 
> ```
> if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
>   ...
> }
> 
> return FD->isInlined() && !Instance.getLangOpts().GNUInline;
> ```
I think this still causes the isInlined + !GNUInlined code to trigger on CXXMethodDecls. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974