Community
Participate
Working Groups
Build ID: I20080617-2000 Steps To Reproduce: 1. --- OtherClass.h --- namespace myName { class OtherClass { }; } --- TestClass.cpp --- #include <TestClass.h> namespace myName { void TestClass::myTest() { OtherClass localVar; } } 2. Go on OtherClass and call "Add Include" 3. The following is added #include <OtherClass.h> using namespace myName; The using directive is unecessary. More information:
Even when some kind of using directive is necessary, Add Include has a choice between "using namespaceName" and "using namespaceName::className". The second option should be the default since it's safer than the first one. Giving user an option to change the default would be an additional bonus.
(In reply to comment #1) > Even when some kind of using directive is necessary, Add Include has a choice > between "using namespaceName" and "using namespaceName::className". The second > option should be the default since it's safer than the first one. Giving user > an option to change the default would be an additional bonus. Hmm, should it add the directive at all? The label 'Add Include' does not indicate that it will change your code (other than adding an include directive). Personally I would want to qualify the name instead of adding the using directive.
(In reply to comment #2) > (In reply to comment #1) > > Even when some kind of using directive is necessary, Add Include has a choice > > between "using namespaceName" and "using namespaceName::className". The second > > option should be the default since it's safer than the first one. Giving user > > an option to change the default would be an additional bonus. > > Hmm, should it add the directive at all? The label 'Add Include' does not > indicate that it will change your code (other than adding an include > directive). Personally I would want to qualify the name instead of adding the > using directive. "Add Include" may not be the best name for this feature. What user really wants is to be able to start using the class or the function. We should think of it more of an assist rather than just an insertion of an include statement. It looks like there are two reasonable alternatives: 1. Qualifying all instances of the name 2. Adding a "using" declaration (not directive) The user should be be able to choose between the two. The first option seems harder to implement than the second one. One possible approach to it is to add a "using" declaration, resolve the names, qualify them, then remove the "using" declaration.
Created attachment 134182 [details] An extensive rewrite of Add Include The patch fixes the insertion of unnecessary 'using' statement and several other issues including bug 236530. Selection of the file to include and the name to use in the include statement has greatly improved. It employs a heuristic algorithm that looks at includes in other source files and tries to follow the prevalent style. The new behavior with respect to 'using' corresponds to option #2 from comment #3. I would like to implement option #1 also, but I'm not sure yet how to efficiently get the list of names that have to be qualified. I'm looking for a solution that doesn't involve a complete re-parsing of the source. Suppose I could iterate through all problem bindings with a particular name and check if they would resolve differently in the presence of a 'using' declaration. Not sure how to do that yet.
Fixed in HEAD > 20090504.
