jrm retitled this revision from compiler.mk: Update clang dependency from v10, clean up duplication to compiler.mk: Update clang dependency from v6 to v10, clean up duplication.

brooks added inline comments.Jan 14 2021, 12:28 AM

Mk/Uses/compiler.mk
174 ↗	(On Diff #82170)	I'd usually put parenthesizes around the && blocks rather than trusting operator precedence.
179 ↗	(On Diff #82170)	Does it make sense to remove this OBE case while you're here (or as a new review)?

jrm added reviewers: linimon, pkubaj.Jan 15 2021, 5:49 PM

jrm added inline comments.

Mk/Uses/compiler.mk
179 ↗	(On Diff #82170)	I lean towards a separate review with some reviewers who work on other architectures, because I don't have access to most of the tier 2+ architectures for testing. Or, are you confident that it's no longer necessary?

brooks added inline comments.Jan 15 2021, 6:03 PM

Mk/Uses/compiler.mk
179 ↗	(On Diff #82170)	Separate review is fine and probably necessary. It's not the case that all Tier 2 arches use clang because sparc64 exists (I keep forgetting it's still around in stable). This should be changed so all supported arches in 13 default to clang, but that will probably require testing.

jrm edited the summary of this revision. (Show Details)Jan 20 2021, 4:44 PM

jrm edited the test plan for this revision. (Show Details)

jrm added inline comments.Jan 20 2021, 4:47 PM

Mk/Uses/compiler.mk
174 ↗	(On Diff #82170)	I just wanted to wait for the exp-run to finish before making any code changes. The exp-run does pass, but are you aware of an issue with operator precedence? Or, do you mean the `( )` improves readability?

brooks added inline comments.Jan 20 2021, 7:28 PM

Mk/Uses/compiler.mk
174 ↗	(On Diff #82170)	IMO `( )` improves readability. I'd rather not have to think about any but the most trivial precedence rules (obviously a subjective thing) when reading large conditionals.