llvm / llvm/circt

[FIRRTL] Investigate LowerClasses namespace lifetimes

Open
#7,329 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement FIRRTL
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

We worked around an issue originally in #7102 and again in #7328.

From @dtzSiFive:

> Doing this lazily is nice, although perhaps as we lean more on these things we're likely to need this for all modules anyway.
>
> I'd like to root this out a bit more thoroughly (is there a move constructor missing? maybe the collection uses unique_ptr's? or just a comment?) but this looks correct and reliable way to fix this with what we have now, thanks!

I'm opening this ticket to track getting to the bottom of the issue so we don't need to pre-allocated the namespaces.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the LowerClasses namespace handling and the workarounds tracked in #7102 and #7328. Trace the namespace lifetimes and ownership to identify why pre-allocation is needed; done means the underlying issue is understood and a reliable resolution is documented or implemented.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.