This is an archive of the discontinued LLVM Phabricator instance.

[TargetParser] Disallow Global Constructors
ClosedPublic

Authored by lenary on Mar 3 2023, 8:31 AM.

Details

Summary

Global Constructors are disallowed in the Support library. The
TargetParser library is likely to go along with the Support library in
most uses, because it contains llvm::Triple, so lets pre-emptively add
the same rule, rather than getting caught out later.

Diff Detail

Event Timeline

lenary created this revision.Mar 3 2023, 8:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2023, 8:31 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
lenary requested review of this revision.Mar 3 2023, 8:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2023, 8:31 AM
lenary added a subscriber: mehdi_amini.

Tagging reviewers. @mehdi_amini I added you as you did the same change to the Support library in rG402461beb051b6a5c158f1e36d8e2c2b676e8804.

@mehdi_amini can you explain what the original reasoning behind this restriction was for Support, and whether we need to make the same restriction for TargetParser now that it's split out?

@mehdi_amini can you explain what the original reasoning behind this restriction was for Support, and whether we need to make the same restriction for TargetParser now that it's split out?

Global ctors/dtors are discouraged/disallowed by the LLVM style guide in general: https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors

So basically, I think, any library that can have the warning/error enabled should - to ensure we conform to this style rule, and don't regress.

tmatheson accepted this revision.Mar 7 2023, 2:45 AM
This revision is now accepted and ready to land.Mar 7 2023, 2:45 AM

My personal motivation was to be able to ship MLIR without any static ctor.
Fixing all of LLVM seemed too much, but MLIR (when not needing the JIT) does only depends on libSupport, which was very much doable (in particular a key piece was that the command line parsing entry point is part of the lib, so we can lazy load libSupport options)

mehdi_amini accepted this revision.Mar 7 2023, 5:31 PM