From 515764d8b55d47ac982e94ec46cf0d2097a16433 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 27 Oct 2021 12:22:55 -0700 Subject: [PATCH] python: Remove incorrect usage of typing 'Optional' There has been some confusion about usage of 'Optional'. In some areas of the codebase it was assumed this specifies an optional parameter (i.e., one which may or may not set, as it has a default value). This is incorrect. 'Optional[]' is shorthand for 'Union[, None]', i.e., it is used to state the value may be 'None'. This patch corrects this throughout the gem5 codebase. Change-Id: I77a6708dee448e8480870d073e128aed3d6ae904 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52143 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/python/gem5/components/boards/x86_board.py | 2 +- .../cachehierarchies/classic/caches/l1dcache.py | 16 ++++++++-------- .../cachehierarchies/classic/caches/l1icache.py | 16 ++++++++-------- .../cachehierarchies/classic/caches/l2cache.py | 16 ++++++++-------- .../classic/caches/mmu_cache.py | 17 +++++++---------- .../cachehierarchies/classic/no_cache.py | 6 ++---- .../classic/private_l1_cache_hierarchy.py | 5 +---- .../private_l1_private_l2_cache_hierarchy.py | 7 ++----- src/python/gem5/resources/downloader.py | 6 +++--- src/python/gem5/resources/resource.py | 2 +- src/python/gem5/utils/requires.py | 2 +- tests/gem5/x86-boot-tests/test_linux_boot.py | 2 +- 12 files changed, 43 insertions(+), 54 deletions(-) diff --git a/src/python/gem5/components/boards/x86_board.py b/src/python/gem5/components/boards/x86_board.py index 49bf7896df..df7e7fb9ee 100644 --- a/src/python/gem5/components/boards/x86_board.py +++ b/src/python/gem5/components/boards/x86_board.py @@ -271,7 +271,7 @@ class X86Board(SimpleBoard): kernel: AbstractResource, disk_image: AbstractResource, command: Optional[str] = None, - kernel_args: Optional[List[str]] = [], + kernel_args: List[str] = [], ): """Setup the full system files diff --git a/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py b/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py index 7346e7a58b..c80032b198 100644 --- a/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py +++ b/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py @@ -28,7 +28,7 @@ from .....utils.override import * from m5.objects import Cache, BasePrefetcher, StridePrefetcher -from typing import Optional, Type +from typing import Type class L1DCache(Cache): @@ -39,13 +39,13 @@ class L1DCache(Cache): def __init__( self, size: str, - assoc: Optional[int] = 8, - tag_latency: Optional[int] = 1, - data_latency: Optional[int] = 1, - response_latency: Optional[int] = 1, - mshrs: Optional[int] = 16, - tgts_per_mshr: Optional[int] = 20, - writeback_clean: Optional[bool] = True, + assoc: int = 8, + tag_latency: int = 1, + data_latency: int = 1, + response_latency: int = 1, + mshrs: int = 16, + tgts_per_mshr: int = 20, + writeback_clean: bool = True, PrefetcherCls: Type[BasePrefetcher] = StridePrefetcher, ): super(L1DCache, self).__init__() diff --git a/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py b/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py index d1bf5aa98e..8e4ba0946f 100644 --- a/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py +++ b/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py @@ -24,7 +24,7 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from typing import Optional, Type +from typing import Type from m5.objects import Cache, BasePrefetcher, StridePrefetcher @@ -39,13 +39,13 @@ class L1ICache(Cache): def __init__( self, size: str, - assoc: Optional[int] = 8, - tag_latency: Optional[int] = 1, - data_latency: Optional[int] = 1, - response_latency: Optional[int] = 1, - mshrs: Optional[int] = 16, - tgts_per_mshr: Optional[int] = 20, - writeback_clean: Optional[bool] = True, + assoc: int = 8, + tag_latency: int = 1, + data_latency: int = 1, + response_latency: int = 1, + mshrs: int = 16, + tgts_per_mshr: int = 20, + writeback_clean: bool = True, PrefetcherCls: Type[BasePrefetcher] = StridePrefetcher, ): super(L1ICache, self).__init__() diff --git a/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py b/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py index f3d7c14c69..b32625527d 100644 --- a/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py +++ b/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py @@ -28,7 +28,7 @@ from .....utils.override import * from m5.objects import Cache, BasePrefetcher, StridePrefetcher -from typing import Optional, Type +from typing import Type class L2Cache(Cache): @@ -39,13 +39,13 @@ class L2Cache(Cache): def __init__( self, size: str, - assoc: Optional[int] = 16, - tag_latency: Optional[int] = 10, - data_latency: Optional[int] = 10, - response_latency: Optional[int] = 1, - mshrs: Optional[int] = 20, - tgts_per_mshr: Optional[int] = 12, - writeback_clean: Optional[bool] = True, + assoc: int = 16, + tag_latency: int = 10, + data_latency: int = 10, + response_latency: int = 1, + mshrs: int = 20, + tgts_per_mshr: int = 12, + writeback_clean: bool = True, PrefetcherCls: Type[BasePrefetcher] = StridePrefetcher, ): super(L2Cache, self).__init__() diff --git a/src/python/gem5/components/cachehierarchies/classic/caches/mmu_cache.py b/src/python/gem5/components/cachehierarchies/classic/caches/mmu_cache.py index 580cb4e966..2048782d8b 100644 --- a/src/python/gem5/components/cachehierarchies/classic/caches/mmu_cache.py +++ b/src/python/gem5/components/cachehierarchies/classic/caches/mmu_cache.py @@ -28,9 +28,6 @@ from .....utils.override import * from m5.objects import Cache, BasePrefetcher, StridePrefetcher -from typing import Optional - - class MMUCache(Cache): """ A simple Memory Management Unit (MMU) cache with default values. @@ -39,13 +36,13 @@ class MMUCache(Cache): def __init__( self, size: str, - assoc: Optional[int] = 4, - tag_latency: Optional[int] = 1, - data_latency: Optional[int] = 1, - response_latency: Optional[int] = 1, - mshrs: Optional[int] = 20, - tgts_per_mshr: Optional[int] = 12, - writeback_clean: Optional[bool] = True, + assoc: int = 4, + tag_latency: int = 1, + data_latency: int = 1, + response_latency: int = 1, + mshrs: int = 20, + tgts_per_mshr: int = 12, + writeback_clean: bool = True, ): super(MMUCache, self).__init__() self.size = size diff --git a/src/python/gem5/components/cachehierarchies/classic/no_cache.py b/src/python/gem5/components/cachehierarchies/classic/no_cache.py index d4f519d629..11f6420bee 100644 --- a/src/python/gem5/components/cachehierarchies/classic/no_cache.py +++ b/src/python/gem5/components/cachehierarchies/classic/no_cache.py @@ -32,8 +32,6 @@ from ....runtime import get_runtime_isa from m5.objects import Bridge, BaseXBar, SystemXBar, BadAddr, Port -from typing import Optional - from ....utils.override import * @@ -76,13 +74,13 @@ class NoCache(AbstractClassicCacheHierarchy): return membus def __init__( - self, membus: Optional[BaseXBar] = _get_default_membus.__func__() + self, membus: BaseXBar = _get_default_membus.__func__() ) -> None: """ :param membus: The memory bus for this setup. This parameter is optional and will default toa 64 bit width SystemXBar is not specified. - :type membus: Optional[BaseXBar] + :type membus: BaseXBar """ super(NoCache, self).__init__() self.membus = membus diff --git a/src/python/gem5/components/cachehierarchies/classic/private_l1_cache_hierarchy.py b/src/python/gem5/components/cachehierarchies/classic/private_l1_cache_hierarchy.py index ce04f46ecc..f1fe5dfbc7 100644 --- a/src/python/gem5/components/cachehierarchies/classic/private_l1_cache_hierarchy.py +++ b/src/python/gem5/components/cachehierarchies/classic/private_l1_cache_hierarchy.py @@ -37,9 +37,6 @@ from m5.objects import Cache, BaseXBar, SystemXBar, BadAddr, Port from ....utils.override import * -from typing import Optional - - class PrivateL1CacheHierarchy(AbstractClassicCacheHierarchy): """ A cache setup where each core has a private L1 data and instruction Cache. @@ -63,7 +60,7 @@ class PrivateL1CacheHierarchy(AbstractClassicCacheHierarchy): self, l1d_size: str, l1i_size: str, - membus: Optional[BaseXBar] = _get_default_membus.__func__(), + membus: BaseXBar = _get_default_membus.__func__(), ) -> None: """ :param l1d_size: The size of the L1 Data Cache (e.g., "32kB"). diff --git a/src/python/gem5/components/cachehierarchies/classic/private_l1_private_l2_cache_hierarchy.py b/src/python/gem5/components/cachehierarchies/classic/private_l1_private_l2_cache_hierarchy.py index cd55c6e5c4..7c4e829a5a 100644 --- a/src/python/gem5/components/cachehierarchies/classic/private_l1_private_l2_cache_hierarchy.py +++ b/src/python/gem5/components/cachehierarchies/classic/private_l1_private_l2_cache_hierarchy.py @@ -39,9 +39,6 @@ from m5.objects import Cache, L2XBar, BaseXBar, SystemXBar, BadAddr, Port from ....utils.override import * -from typing import Optional - - class PrivateL1PrivateL2CacheHierarchy( AbstractClassicCacheHierarchy, AbstractTwoLevelCacheHierarchy ): @@ -71,7 +68,7 @@ class PrivateL1PrivateL2CacheHierarchy( l1d_size: str, l1i_size: str, l2_size: str, - membus: Optional[BaseXBar] = _get_default_membus.__func__(), + membus: BaseXBar = _get_default_membus.__func__(), ) -> None: """ :param l1d_size: The size of the L1 Data Cache (e.g., "32kB"). @@ -89,7 +86,7 @@ class PrivateL1PrivateL2CacheHierarchy( :param membus: The memory bus. This parameter is optional parameter and will default to a 64 bit width SystemXBar is not specified. - :type membus: Optional[BaseXBar] + :type membus: BaseXBar """ AbstractClassicCacheHierarchy.__init__(self=self) diff --git a/src/python/gem5/resources/downloader.py b/src/python/gem5/resources/downloader.py index f05d9d098f..fab7864c56 100644 --- a/src/python/gem5/resources/downloader.py +++ b/src/python/gem5/resources/downloader.py @@ -32,7 +32,7 @@ import shutil import gzip import hashlib import base64 -from typing import List, Dict, Optional +from typing import List, Dict from ..utils.filelock import FileLock @@ -197,8 +197,8 @@ def get_resources_json_obj(resource_name: str) -> Dict: def get_resource( resource_name: str, to_path: str, - unzip: Optional[bool] = True, - override: Optional[bool] = False, + unzip: bool = True, + override: bool = False, ) -> None: """ Obtains a gem5 resource and stored it to a specified location. If the diff --git a/src/python/gem5/resources/resource.py b/src/python/gem5/resources/resource.py index bf83e0a16a..07c6a80b25 100644 --- a/src/python/gem5/resources/resource.py +++ b/src/python/gem5/resources/resource.py @@ -85,7 +85,7 @@ class Resource(AbstractResource): self, resource_name: str, resource_directory: Optional[str] = None, - override: Optional[bool] = False, + override: bool = False, ): """ :param resource_name: The name of the gem5 resource. diff --git a/src/python/gem5/utils/requires.py b/src/python/gem5/utils/requires.py index 2fa6c905f0..76a8b41393 100644 --- a/src/python/gem5/utils/requires.py +++ b/src/python/gem5/utils/requires.py @@ -53,7 +53,7 @@ def _get_exception_str(msg: str): def requires( isa_required: Optional[ISA] = None, coherence_protocol_required: Optional[CoherenceProtocol] = None, - kvm_required: Optional[bool] = False, + kvm_required: bool = False, ) -> None: """ Ensures the ISA/Coherence protocol/KVM requirements are met. An exception diff --git a/tests/gem5/x86-boot-tests/test_linux_boot.py b/tests/gem5/x86-boot-tests/test_linux_boot.py index 124ea8e170..c513f4050c 100644 --- a/tests/gem5/x86-boot-tests/test_linux_boot.py +++ b/tests/gem5/x86-boot-tests/test_linux_boot.py @@ -40,7 +40,7 @@ def test_boot( num_cpus: int, mem_system: str, length: str, - boot_type: Optional[str] = "init", + boot_type: str = "init", to_tick: Optional[int] = None, ):