rjmccall added a comment.

@rsmith This is a big enough architectural change that I'd appreciate your 
sign-off on the basic approach.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:3759
   unsigned BuiltinID;
-  if (Old->isImplicit() && (BuiltinID = Old->getBuiltinID())) {
+  bool RevertedBuiltin{Old->getIdentifier()->hasRevertedBuiltin()};
+  if (Old->isImplicit() &&
----------------
rjmccall wrote:
> The old code didn't check this eagerly.  The `Old->isImplicit()` check is 
> unlikely to fire; please make sure that we don't do any extra work unless it 
> does.
Since BuiltinAttr is inherited, we should keep the isImplicit check, because we 
only want to use a special diagnostic when "redeclaring" the implicit builtin 
declaration.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:8880
+      }
+    }
+
----------------
Hmm.  I'm concerned about not doing any sort of semantic compatibility check 
here before we assign the function special semantics.  Have we really never 
done those in C++?

If not, I wonder if we can reasonably make an implicit declaration but just 
make it hidden from lookup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to