chipsalliance / chipsalliance/fasm

Remove `add_flags` method in setup.py by fixing cmake config file

Open
#50 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
104
Forks
35
PR merge metrics
No merged PRs in 30d

Description

The pull request at #48 adds a `add_flags` method. These flags should be instead correctly set via detection in the cmake file.

The code is shown below;
```python
def add_flags(self):
if sys.platform.startswith('win'):
return

for flag in ["CFLAGS", "CXXFLAGS"]:
flags = [os.environ.get(flag, "")]
if not flags[0]:
flags.pop(0)

if shared_options.antlr_runtime == 'static':
# When linking the ANTLR runtime statically, -fPIC is
# still necessary because libparse_fasm will be a
# shared library.
flags.append("-fPIC")

# FIXME: These should be in the cmake config file?
# Disable excessive warnings currently in ANTLR runtime.
# warning: type attributes ignored after type is already defined
# `class ANTLR4CPP_PUBLIC ATN;`
flags.append('-Wno-attributes')

# Lots of implicit fallthroughs.
# flags.append('-Wimplicit-fallthrough=0')

if flags:
os.environ[flag] = " ".join(flags)
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Read setup.py and the CMake configuration responsible for compiler flags, starting with the add_flags method shown in the issue. Done means removing add_flags and moving its flag detection and ANTLR-related settings into the CMake configuration; no tests are mentioned in the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, python
Domain
build-system
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.