plotfi marked 6 inline comments as done.
plotfi added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:626
HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_ifso : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>,
Group<Action_Group>,
+ HelpText<"Generate Inteface Stub Files.">;
----------------
compnerd wrote:
> I thought that we were going to add `experimental` to this for the time being?
Oh, I was specifying experimental in the
-interface-stubs-version=experimental-ifo-elf-v1. Do you want the main flag to
also be -emit-interface-stubs-experimental??
================
Comment at: clang/include/clang/Driver/Types.def:91
TYPE("ast", AST, INVALID, "ast", "u")
+TYPE("ifs", IFS, INVALID, "ifs", "u")
TYPE("pcm", ModuleFile, INVALID, "pcm", "u")
----------------
compnerd wrote:
> What about `ifo` instead of `ifs`?
I went with ifs because ifo implies the analog of a .o file and ifso implies
the analog of a .so file. I want the tbe/ifs text files to just be thought of
as something a little different. Like symbol listings that another tool can
assemble into whatever format.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1649
+ case OPT_emit_ifso:
+ Opts.ProgramAction = frontend::GenerateIfsoTbeExpV1;
+ if (Args.hasArg(OPT_ifso_version_EQ))
----------------
compnerd wrote:
> Can we make this required to be specified instead of defaulting please? I
> think that it is more clear.
I think that makes sense.
================
Comment at: clang/lib/Frontend/FrontendActions.cpp:179
+ };
+ using MangledSymbols = std::map<const NamedDecl *, MangledSymbol>;
+
----------------
compnerd wrote:
> Hmm, I wonder if `DenseSet` is more appropriate here. Is there some reason
> that I am overlooking for supporting duplicate entries? In fact, since the
> key is a pointer, you could even do a `DenseMap`.
Can we do data-structure improvements post landing the first version?
================
Comment at: clang/lib/Frontend/FrontendActions.cpp:214
+ ND->dump();
+ }
+ return true;
----------------
compnerd wrote:
> debugging left over?
Ah yeah I need to replace this with an llvm_unreachable or something.
================
Comment at: clang/lib/Frontend/FrontendActions.cpp:297
+ sema.LateTemplateParser(sema.OpaqueParser, LPT);
+ HandleNamedDecl(FD, Symbols, (FromTU | IsLate));
+ }
----------------
compnerd wrote:
> Typo of `||`? The field is boolean not a mask.
It is a bitmask.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60974/new/
https://reviews.llvm.org/D60974
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
