[PATCH] D112059: Fix inline builtin handling in case of redefinition

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added subscribers: aaron.ballman, rsmith.
nickdesaulniers added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302
   // in case the function pointer is referenced somewhere.
-  if (FD->isInlineBuiltinDeclaration() && Fn) {
+  if (Fn) {
     std::string FDInlineName = (Fn->getName() + ".inline").str();
----------------
I don't think we want to do all this work if just `Fn`; ie. create a new `std::string` with `.inline` suffix for every function we're going to generate code (IR) for.  How about we add an additional unlikely guard:

`if (FD->getBuiltinID() && FN) {`

Because in the usual case, `FD` both has a builtin ID and is an inline builtin declaration, while in the exceptional case that this patch addresses, `FD` has a builtin ID but is not an inline builtin declaration.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1319-1321
+    // everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
+    // to detect that situation before we reach codegen, so do some late
+    // replacement.
----------------
Perhaps in `Sema::CheckFunctionDeclaration`?  I see there is where we detect redeclarations. The calls from there to `FunctionDecl::setPreviousDeclaration()` seem to set up the redecl chain.  Perhaps this exceptional case (or both cases, even) would be handled better there?

cc @rsmith @aaron.ballman in case they have feedback/tips/cycles.


================
Comment at: clang/test/CodeGen/strlen-inline-builtin-redecl.c:8
+unsigned long strnlen(const char *, unsigned long);
+void fortify_panic(const char *);
+
----------------
unused decl


================
Comment at: clang/test/CodeGen/strlen-inline-builtin-redecl.c:10
+
+extern inline __attribute__((always_inline)) __attribute__((gnu_inline)) unsigned long strlen(const char *p) {
+  return 1;
----------------
Do you mind wrapping this to 80 chars wide? I suspect if you put the two `__attribute__`s first, then the formatter will do a better job.  You can also combine these, a la `__attribute__((always_inline, gnu_inline))` to cut down on line length.


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

https://reviews.llvm.org/D112059