Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the Clang tool module to remove a redundant assignment to the c++_static map when a static runtime is used. A review comment suggests refining the conditional check to specifically target stdc++_static, which would improve precision and provide better safety against null values.
| @@ -285,7 +285,6 @@ function nf_runtime(self, runtime, opt) | |||
| end | |||
| end | |||
| if runtime:endswith("_static") and _has_static_libstdcxx(self) then | |||
There was a problem hiding this comment.
The condition runtime:endswith("_static") matches both c++_static and stdc++_static. Since the logic inside this block now only updates maps["stdc++_static"], it is more precise to check for runtime == "stdc++_static". This avoids unnecessary calls to _has_static_libstdcxx(self) (which may involve compiler flag detection) when runtime is c++_static. Additionally, using == is safer as it prevents a potential crash if runtime is nil, whereas endswith would throw an error.
if runtime == "stdc++_static" and _has_static_libstdcxx(self) then
#7442