search

avr-llvm / llvm

[MERGED UPSTREAM] AVR backend for the LLVM compiler library
220 stars 21 forks source link

Rename and reformat #117

Closed agnat closed 9 years ago

agnat commented 9 years ago

Improve readability...

agnat commented 9 years ago

Fixed.

dylanmckay commented 9 years ago

PseudoSubtargetFeature always did feel like a hack, FeatureSet is much better!

You've refactored #includes so that the general LLVM files are listed first, and the AVR-specific headers are next, separated by an empty line. I like this. We should write up a coding guidelines page on the Wiki and include this.

Rollup and I'll merge.

dylanmckay commented 9 years ago

Also, what is your opinion on #pragma once? I'd rather use this than #include guards, but it seems the rest of LLVM does not, which makes me hesitate.

Once the backend is mostly bug free, I'd like to attempt to get it included in LLVM master, so we may end up having to change it somewhere down the line?

The LLVM coding standards document doesn't mention them.

agnat commented 9 years ago

Also, what is your opinion on #pragma once?

I use include guards. They work just fine. They are standard ... no need to change that habit.

[...] I'd like to attempt to get it included in LLVM master

Attempt? What do you mean "attempt"? We will go there and land it.

We will also explain why we break the include style and why our ordering is clearly superior. Other than that it will conform to the standard of course.

Rollup and I'll merge.

Done.