PLEASE REVIEW: modularize: new preprocessor conditional directive checking feature

Thompson, John John_Thompson at playstation.sony.com
Thu Jun 20 18:41:24 PDT 2013


Sean,

Thanks for your efforts in this.  As you can see, I'm struggling to become comfortable with this style of coding and using STL, but with your help I'm starting to learn.

>Again, this function is not appropriate. Please investigate the implementation of clang's macro-related diagnostic messages for inspiration, or consult with cfe-dev with a more refined question than your last question (see below; I think I may have identified what the key question is); the appropriate solution will almost surely require significant changes to your patch. Clang gives detailed notes in diagnostics if the offending code was expanded from a macro, and traces the expansion many levels deep in some cases.

I will continue to look into this tomorrow.  Today I just pretty much rewrote a lot of the code per your comments about style and conventions.

Previously you mentioned that I should make smaller patches, which implies an iterative approach.  Please consider this a step in the process.  The bottom line is that even though this approach is flawed, it nonetheless mostly works.  Note that having the preprocessed conditional is primarily for the benefit of the user in displaying the message to help him understand which macro is different.  An alternative would have been to modify the PPCallbacks mechanism to pass along a flag indicating the resulting condition Boolean value, rather than my using a string compare.  The fact that it didn't pass along this flag seems to be an oversight, but I can fix it in a separate patch.  Even though the preprocessed output isn't correct in the case of function macros, because it still incidentally likely turns out different when a macro is used or a macro argument is passed, it's still valid as a test for comparing the conditions.

If you find more style and convention problems, please do pass them along and I will address them before checking in.  But please let me check this in as a functional step in the process.  After all, this is still an experimental tool, and I think it would be better to make incremental changes, starting with this as a baseline.

Thanks for the reference to clang-format.  It's made things much easier.

The latest patch is enclosed.

-John

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130621/638a82cc/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mod_2013_06_20_patch.txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130621/638a82cc/attachment.txt>


More information about the cfe-commits mailing list