[PATCH] D104777: PR50767: clear non-distinct debuginfo for function with nodebug definition after undecorated declaration

[Bruno De Fraine via Phabricator via cfe-commits]

brunodefraine added a comment.

Following suggestion by @dblaikie and @aaron.ballman I'm preparing a new diff where a function redeclaration that adds `nodebug` is flagged as an error in `Sema::mergeDeclAttributes`.

This triggers test failures because of violations in builtin header files, for example:

  In file included from clang/test/CodeGen/ms-intrinsics.c:18:
  build/lib/clang/13.0.0/include/intrin.h:451:24: error: function declared with 'nodebug' attribute was previously declared without the 'nodebug' attribute
  static __inline__ void __DEFAULT_FN_ATTRS __movsb(unsigned char *__dst,
                         ^
  build/lib/clang/13.0.0/include/intrin.h:37:62: note: expanded from macro '__DEFAULT_FN_ATTRS'
  #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
                                                               ^
  build/lib/clang/13.0.0/include/intrin.h:80:6: note: previous declaration is here
  void __movsb(unsigned char *, unsigned char const *, size_t);
       ^

>From what I understand from `clang/lib/Headers/intrin.h` the functions are declared in general at the beginning of the headers, then implemented for specific architectures as inline function containing assembly. I do not know if it is appropriate to mark the general declarations as `nodebug`...

That I already encounter this in builtin headers is probably an indication that the new error will break some code in the wild. A pragmatic solution would be to only raise the error in `Sema::mergeDeclAttributes` if `Old->isUsed()`, i.e. an earlier declaration without `nodebug` can be forgiven if not yet used. I think that would still make it impossible to trigger the crash?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104777