Generate compiler flags from toml file

[jjgarzella]

Fixes #119

Description:
Moved the flags from the Makefile to the toml file.

In previous versions of FLiT, each flag had a variable associated with it. These variable names are now auto-generated by Python and added to the Makefile. In particular, this means that

1. The names of the variables in the Makefile are not abbreviated, and thus much longer.
2. Switches and optimization levels are no longer guaranteed to be executed in any order.

Documentation:
What documentation did you change?
There should be no changes to documentation, the changes were relatively simple.

Tests:
What automated tests did you change?
All of the previous tests should pass.

[mikebentley15]

Thanks for this pull request JJ. I started working on this issue yesterday as well. I'd be happy to merge together our efforts. For example, I've updated the documentation.

I like the approach you are doing to generate Makefile variable names for the flags. I was going to do a simple approach of CLANG_SWITCH01 and just enumerate them up from there. I'll take a look at the generated Makefile from your approach. I think it is important that the generated Makefile be at least readable and understandable.

One major missing component for this pull request is the lack of automated tests to see if the flags from the configuration file are actually used and nothing else. I think I have an idea of how to test that, but remember that pull requests must have associated documentation and tests. That being said, it's fine to have the code change only when you make the pull request to get feedback on the type of change you propose before having to make automated tests and documentation updates.

[mikebentley15]

As an aside, if you plan to work on an issue, please comment so on that issue so that in the future we do not duplicate efforts. I would have worked on something else had I known that you were working on issue #119.

[jjgarzella]

Yes, what you said about getting feedback is exactly why I wanted to submit the pull request without having documentation and testing. In addition, how exactly to update documentation and testing are unclear to me, so I figured it would be best to PR and get feedback and direction.

I'm all ears to your testing approach, I have no idea how to do it.

Sorry to duplicate work, I thought that I had communicated that I was going to work on this per conversations in previous weeks, but obviously I wasn't clear enough. I'll do better in the future.

[mikebentley15]

You may be right about our conversations. My memory isn't so great... It was probably me who dropped the ball. For this reason, it may be good to have it documented in the issue at the moment you start working on it.

[mikebentley15]

I created a pull request on your branch feature/flags_toml. It is the baseline before merging our changes. At your earliest convenience, please quickly review and merge it in.

I am done with the merging change. It just needs some automated tests to complete it. As soon as you are finished with the current pull request on your fork, I will create a new pull request to your same feature branch with my changes.

[mikebentley15]

Tests that I think are needed:

1. Test that not specifying the [[compiler]] section gives the default values
2. Test that specifying a compiler, but not specifying the optimization levels gives the default values
3. Test that specifying a compiler, but not specifying the switches list gives the default values
4. Test that the provided list of optimization levels and switches are used, and nothing more.
5. Test that by only specifying one or two compilers, only those specified are used
6. Test error cases, such as specifying more than one type of a compiler

Can you think of any other corner cases?

[jjgarzella]

Ok, it's merged and ready for the second PR. I can't think of any corner cases, the cases I can think of will be applicable when more compiler types are supported.

[jjgarzella]

Do you really think that the empty string should be required to be in the toml file, as it is now? In my opinion, that option should be automatically generated and run by FLiT, as I can't think of a time when I wouldn't want that to be a use case. Moreover, it isn't great that it is required to be at the end, that could confuse users.

[mikebentley15]

I agree that it is annoying to have at the end of the list without a comma. That is only necessary if they want their configuration file to work with the older buggy version of python-toml (for which I submitted a pull request on their repository and got it fixed). But yeah, if they do want to support or they have the old buggy version, then they need to have it at the end, and it is confusing for users (it was sure confusing to me when my files failed to parse).

As for the empty string being there, you may be right that there may not be many situations where you don't want to run that, but I think it is more confusing when tools automatically add things that I do not explicitly specify. Even if every user has to specify it, I prefer it to be explicit rather than implicit and be required to heavily document this.

I can think of one use case that may be common. Consider you want to use FLiT to search between three different libraries. They have different interfaces, so you use #ifdef macros everywhere based on defined macro variables. Every compilation must have exactly one of these three library macros defined, and so each entry in the switches_list would have exactly one of these defined (along with maybe other flags). In such a case, having no switches as a possibility would break things since none of the libraries are used. Did that use case make sense?

I appreciate your opinion on the matter. Thank you.

[jjgarzella]

Ok, that use case make enough sense to merit the feature the way it is. This needs to be documented, though, but that goes without saying ;)

[mikebentley15]

Yeah, fair enough

[mikebentley15]

I created a pull request on your branch feature/flags_toml. It is the set of automated tests for this change.

I made another issue (#248) to cover the other automated tests needed for flit update and the configuration file in general.

[mikebentley15]

I'll just merge in the other changes with a new pull request. To complete this one, I will simply merge it into my feature branch for this change.