This is an archive of the discontinued LLVM Phabricator instance.

[NFCI] Move DEBUG_TYPE definition below #include of headers
ClosedPublic

Authored by mdchen on May 16 2021, 11:55 PM.

Details

Summary

There're several places in the code base where DEBUG_TYPE definition is defined at the top of the file or around the #includes, which could result in conflicts when you try to define a new DEBUG_TYPE in a related header file.

Diff Detail

Event Timeline

mdchen created this revision.May 16 2021, 11:55 PM
mdchen requested review of this revision.May 16 2021, 11:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2021, 11:55 PM

This fix looks good.

which could result in conflicts when you try to define a new DEBUG_TYPE in a related header file.

And I am wondering the reason you want to define DEBUG_TYPE in a header file. It isn't what we used to do.

And I am wondering the reason you want to define DEBUG_TYPE in a header file. It isn't what we used to do.

I don't think it's true. We can find many header files under llvm/include/ have their own DEBUG_TYPE, and it works well if you carefully undefine it at the end of the file.

And I am wondering the reason you want to define DEBUG_TYPE in a header file. It isn't what we used to do.

I don't think it's true. We can find many header files under llvm/include/ have their own DEBUG_TYPE, and it works well if you carefully undefine it at the end of the file.

Yeah, the codes shows you are right. I find definitions for DEBUG_TYPE in some headers.

mdchen updated this revision to Diff 345807.May 17 2021, 3:11 AM

Did a fully scan and modified more files that have the same issue.

tejohnson accepted this revision.May 18 2021, 8:25 AM

lgtm. this seems fine to me, but presumably we would have a compile error if there was ever a conflict in practice with one of these files?

This revision is now accepted and ready to land.May 18 2021, 8:25 AM

lgtm. this seems fine to me, but presumably we would have a compile error if there was ever a conflict in practice with one of these files?

In practice it conflicts with macros defined in header files; if a conflict header file defines one but doesn't undefined it(which is unusual), the compiler throws a redefinition warning; in a general case, there'll be a compile error instead. Thanks for the review!

This revision was landed with ongoing or failed builds.May 30 2021, 2:32 AM
This revision was automatically updated to reflect the committed changes.